From: Eric Auger <eric.auger@redhat.com>
Let the vfio-ccw device use vfio_attach_device() and
vfio_detach_device(), hence hiding the details of the used
IOMMU backend.
Also now all the devices have been migrated to use the new
vfio_attach_device/vfio_detach_device API, let's turn the
legacy functions into static functions, local to container.c.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
hw/vfio/ccw.c | 120 ++++++++--------------------------
hw/vfio/container.c | 10 +--
include/hw/vfio/vfio-common.h | 5 --
3 files changed, 33 insertions(+), 102 deletions(-)
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 1e2fce83b0..f078e014fa 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -572,88 +572,15 @@ static void vfio_ccw_put_region(VFIOCCWDevice *vcdev)
g_free(vcdev->io_region);
}
-static void vfio_ccw_put_device(VFIOCCWDevice *vcdev)
-{
- g_free(vcdev->vdev.name);
- vfio_put_base_device(&vcdev->vdev);
-}
-
-static void vfio_ccw_get_device(VFIOGroup *group, VFIOCCWDevice *vcdev,
- Error **errp)
-{
- S390CCWDevice *cdev = S390_CCW_DEVICE(vcdev);
- char *name = g_strdup_printf("%x.%x.%04x", cdev->hostid.cssid,
- cdev->hostid.ssid,
- cdev->hostid.devid);
- VFIODevice *vbasedev;
-
- QLIST_FOREACH(vbasedev, &group->device_list, next) {
- if (strcmp(vbasedev->name, name) == 0) {
- error_setg(errp, "vfio: subchannel %s has already been attached",
- name);
- goto out_err;
- }
- }
-
- /*
- * All vfio-ccw devices are believed to operate in a way compatible with
- * discarding of memory in RAM blocks, ie. pages pinned in the host are
- * in the current working set of the guest driver and therefore never
- * overlap e.g., with pages available to the guest balloon driver. This
- * needs to be set before vfio_get_device() for vfio common to handle
- * ram_block_discard_disable().
- */
- vcdev->vdev.ram_block_discard_allowed = true;
-
- if (vfio_get_device(group, cdev->mdevid, &vcdev->vdev, errp)) {
- goto out_err;
- }
-
- vcdev->vdev.ops = &vfio_ccw_ops;
- vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW;
- vcdev->vdev.name = name;
- vcdev->vdev.dev = DEVICE(vcdev);
-
- return;
-
-out_err:
- g_free(name);
-}
-
-static VFIOGroup *vfio_ccw_get_group(S390CCWDevice *cdev, Error **errp)
-{
- char *tmp, group_path[PATH_MAX];
- ssize_t len;
- int groupid;
-
- tmp = g_strdup_printf("/sys/bus/css/devices/%x.%x.%04x/%s/iommu_group",
- cdev->hostid.cssid, cdev->hostid.ssid,
- cdev->hostid.devid, cdev->mdevid);
- len = readlink(tmp, group_path, sizeof(group_path));
- g_free(tmp);
-
- if (len <= 0 || len >= sizeof(group_path)) {
- error_setg(errp, "vfio: no iommu_group found");
- return NULL;
- }
-
- group_path[len] = 0;
-
- if (sscanf(basename(group_path), "%d", &groupid) != 1) {
- error_setg(errp, "vfio: failed to read %s", group_path);
- return NULL;
- }
-
- return vfio_get_group(groupid, &address_space_memory, errp);
-}
-
static void vfio_ccw_realize(DeviceState *dev, Error **errp)
{
- VFIOGroup *group;
- S390CCWDevice *cdev = S390_CCW_DEVICE(dev);
- VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
+ CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
+ S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj, ccw_dev);
+ VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev);
+ VFIODevice *vbasedev = &vcdev->vdev;
Error *err = NULL;
+ int ret;
/* Call the class init function for subchannel. */
if (cdc->realize) {
@@ -663,14 +590,25 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp)
}
}
- group = vfio_ccw_get_group(cdev, &err);
- if (!group) {
- goto out_group_err;
- }
+ vbasedev->ops = &vfio_ccw_ops;
+ vbasedev->type = VFIO_DEVICE_TYPE_CCW;
+ vbasedev->name = g_strdup(cdev->mdevid);
+ vbasedev->dev = &vcdev->cdev.parent_obj.parent_obj;
- vfio_ccw_get_device(group, vcdev, &err);
- if (err) {
- goto out_device_err;
+ /*
+ * All vfio-ccw devices are believed to operate in a way compatible with
+ * discarding of memory in RAM blocks, ie. pages pinned in the host are
+ * in the current working set of the guest driver and therefore never
+ * overlap e.g., with pages available to the guest balloon driver. This
+ * needs to be set before vfio_get_device() for vfio common to handle
+ * ram_block_discard_disable().
+ */
+ vbasedev->ram_block_discard_allowed = true;
+
+ ret = vfio_attach_device(vbasedev->name, vbasedev,
+ &address_space_memory, errp);
+ if (ret) {
+ goto out_attach_dev_err;
}
vfio_ccw_get_region(vcdev, &err);
@@ -708,10 +646,9 @@ out_irq_notifier_err:
out_io_notifier_err:
vfio_ccw_put_region(vcdev);
out_region_err:
- vfio_ccw_put_device(vcdev);
-out_device_err:
- vfio_put_group(group);
-out_group_err:
+ vfio_detach_device(vbasedev);
+out_attach_dev_err:
+ g_free(vbasedev->name);
if (cdc->unrealize) {
cdc->unrealize(cdev);
}
@@ -724,14 +661,13 @@ static void vfio_ccw_unrealize(DeviceState *dev)
S390CCWDevice *cdev = S390_CCW_DEVICE(dev);
VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev);
- VFIOGroup *group = vcdev->vdev.group;
vfio_ccw_unregister_irq_notifier(vcdev, VFIO_CCW_REQ_IRQ_INDEX);
vfio_ccw_unregister_irq_notifier(vcdev, VFIO_CCW_CRW_IRQ_INDEX);
vfio_ccw_unregister_irq_notifier(vcdev, VFIO_CCW_IO_IRQ_INDEX);
vfio_ccw_put_region(vcdev);
- vfio_ccw_put_device(vcdev);
- vfio_put_group(group);
+ vfio_detach_device(&vcdev->vdev);
+ g_free(vcdev->vdev.name);
if (cdc->unrealize) {
cdc->unrealize(cdev);
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 74556da0c7..c71fddc09a 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -837,7 +837,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
}
}
-VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp)
+static VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp)
{
VFIOGroup *group;
char path[32];
@@ -900,7 +900,7 @@ free_group_exit:
return NULL;
}
-void vfio_put_group(VFIOGroup *group)
+static void vfio_put_group(VFIOGroup *group)
{
if (!group || !QLIST_EMPTY(&group->device_list)) {
return;
@@ -917,8 +917,8 @@ void vfio_put_group(VFIOGroup *group)
g_free(group);
}
-int vfio_get_device(VFIOGroup *group, const char *name,
- VFIODevice *vbasedev, Error **errp)
+static int vfio_get_device(VFIOGroup *group, const char *name,
+ VFIODevice *vbasedev, Error **errp)
{
g_autofree struct vfio_device_info *info = NULL;
int fd;
@@ -976,7 +976,7 @@ int vfio_get_device(VFIOGroup *group, const char *name,
return 0;
}
-void vfio_put_base_device(VFIODevice *vbasedev)
+static void vfio_put_base_device(VFIODevice *vbasedev)
{
if (!vbasedev->group) {
return;
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index a29dfe7723..95bcafdaf6 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -230,7 +230,6 @@ int vfio_container_add_section_window(VFIOContainer *container,
void vfio_container_del_section_window(VFIOContainer *container,
MemoryRegionSection *section);
-void vfio_put_base_device(VFIODevice *vbasedev);
void vfio_disable_irqindex(VFIODevice *vbasedev, int index);
void vfio_unmask_single_irqindex(VFIODevice *vbasedev, int index);
void vfio_mask_single_irqindex(VFIODevice *vbasedev, int index);
@@ -248,11 +247,7 @@ void vfio_region_unmap(VFIORegion *region);
void vfio_region_exit(VFIORegion *region);
void vfio_region_finalize(VFIORegion *region);
void vfio_reset_handler(void *opaque);
-VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp);
-void vfio_put_group(VFIOGroup *group);
struct vfio_device_info *vfio_get_device_info(int fd);
-int vfio_get_device(VFIOGroup *group, const char *name,
- VFIODevice *vbasedev, Error **errp);
int vfio_attach_device(char *name, VFIODevice *vbasedev,
AddressSpace *as, Error **errp);
void vfio_detach_device(VFIODevice *vbasedev);
--
2.34.1On 8/30/23 12:37, Zhenzhong Duan wrote:
> From: Eric Auger <eric.auger@redhat.com>
>
> Let the vfio-ccw device use vfio_attach_device() and
> vfio_detach_device(), hence hiding the details of the used
> IOMMU backend.
>
> Also now all the devices have been migrated to use the new
> vfio_attach_device/vfio_detach_device API, let's turn the
> legacy functions into static functions, local to container.c.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Zhenzhong,
Could you please resend 1-12 independantly as a prereq series for iommufd
support ? I think there wouldn't be much to say and they could be merged
pretty quickly.
Thanks,
C.
> ---
> hw/vfio/ccw.c | 120 ++++++++--------------------------
> hw/vfio/container.c | 10 +--
> include/hw/vfio/vfio-common.h | 5 --
> 3 files changed, 33 insertions(+), 102 deletions(-)
>
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 1e2fce83b0..f078e014fa 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -572,88 +572,15 @@ static void vfio_ccw_put_region(VFIOCCWDevice *vcdev)
> g_free(vcdev->io_region);
> }
>
> -static void vfio_ccw_put_device(VFIOCCWDevice *vcdev)
> -{
> - g_free(vcdev->vdev.name);
> - vfio_put_base_device(&vcdev->vdev);
> -}
> -
> -static void vfio_ccw_get_device(VFIOGroup *group, VFIOCCWDevice *vcdev,
> - Error **errp)
> -{
> - S390CCWDevice *cdev = S390_CCW_DEVICE(vcdev);
> - char *name = g_strdup_printf("%x.%x.%04x", cdev->hostid.cssid,
> - cdev->hostid.ssid,
> - cdev->hostid.devid);
> - VFIODevice *vbasedev;
> -
> - QLIST_FOREACH(vbasedev, &group->device_list, next) {
> - if (strcmp(vbasedev->name, name) == 0) {
> - error_setg(errp, "vfio: subchannel %s has already been attached",
> - name);
> - goto out_err;
> - }
> - }
> -
> - /*
> - * All vfio-ccw devices are believed to operate in a way compatible with
> - * discarding of memory in RAM blocks, ie. pages pinned in the host are
> - * in the current working set of the guest driver and therefore never
> - * overlap e.g., with pages available to the guest balloon driver. This
> - * needs to be set before vfio_get_device() for vfio common to handle
> - * ram_block_discard_disable().
> - */
> - vcdev->vdev.ram_block_discard_allowed = true;
> -
> - if (vfio_get_device(group, cdev->mdevid, &vcdev->vdev, errp)) {
> - goto out_err;
> - }
> -
> - vcdev->vdev.ops = &vfio_ccw_ops;
> - vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW;
> - vcdev->vdev.name = name;
> - vcdev->vdev.dev = DEVICE(vcdev);
> -
> - return;
> -
> -out_err:
> - g_free(name);
> -}
> -
> -static VFIOGroup *vfio_ccw_get_group(S390CCWDevice *cdev, Error **errp)
> -{
> - char *tmp, group_path[PATH_MAX];
> - ssize_t len;
> - int groupid;
> -
> - tmp = g_strdup_printf("/sys/bus/css/devices/%x.%x.%04x/%s/iommu_group",
> - cdev->hostid.cssid, cdev->hostid.ssid,
> - cdev->hostid.devid, cdev->mdevid);
> - len = readlink(tmp, group_path, sizeof(group_path));
> - g_free(tmp);
> -
> - if (len <= 0 || len >= sizeof(group_path)) {
> - error_setg(errp, "vfio: no iommu_group found");
> - return NULL;
> - }
> -
> - group_path[len] = 0;
> -
> - if (sscanf(basename(group_path), "%d", &groupid) != 1) {
> - error_setg(errp, "vfio: failed to read %s", group_path);
> - return NULL;
> - }
> -
> - return vfio_get_group(groupid, &address_space_memory, errp);
> -}
> -
> static void vfio_ccw_realize(DeviceState *dev, Error **errp)
> {
> - VFIOGroup *group;
> - S390CCWDevice *cdev = S390_CCW_DEVICE(dev);
> - VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
> + CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
> + S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj, ccw_dev);
> + VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
> S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev);
> + VFIODevice *vbasedev = &vcdev->vdev;
> Error *err = NULL;
> + int ret;
>
> /* Call the class init function for subchannel. */
> if (cdc->realize) {
> @@ -663,14 +590,25 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp)
> }
> }
>
> - group = vfio_ccw_get_group(cdev, &err);
> - if (!group) {
> - goto out_group_err;
> - }
> + vbasedev->ops = &vfio_ccw_ops;
> + vbasedev->type = VFIO_DEVICE_TYPE_CCW;
> + vbasedev->name = g_strdup(cdev->mdevid);
> + vbasedev->dev = &vcdev->cdev.parent_obj.parent_obj;
>
> - vfio_ccw_get_device(group, vcdev, &err);
> - if (err) {
> - goto out_device_err;
> + /*
> + * All vfio-ccw devices are believed to operate in a way compatible with
> + * discarding of memory in RAM blocks, ie. pages pinned in the host are
> + * in the current working set of the guest driver and therefore never
> + * overlap e.g., with pages available to the guest balloon driver. This
> + * needs to be set before vfio_get_device() for vfio common to handle
> + * ram_block_discard_disable().
> + */
> + vbasedev->ram_block_discard_allowed = true;
> +
> + ret = vfio_attach_device(vbasedev->name, vbasedev,
> + &address_space_memory, errp);
> + if (ret) {
> + goto out_attach_dev_err;
> }
>
> vfio_ccw_get_region(vcdev, &err);
> @@ -708,10 +646,9 @@ out_irq_notifier_err:
> out_io_notifier_err:
> vfio_ccw_put_region(vcdev);
> out_region_err:
> - vfio_ccw_put_device(vcdev);
> -out_device_err:
> - vfio_put_group(group);
> -out_group_err:
> + vfio_detach_device(vbasedev);
> +out_attach_dev_err:
> + g_free(vbasedev->name);
> if (cdc->unrealize) {
> cdc->unrealize(cdev);
> }
> @@ -724,14 +661,13 @@ static void vfio_ccw_unrealize(DeviceState *dev)
> S390CCWDevice *cdev = S390_CCW_DEVICE(dev);
> VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
> S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev);
> - VFIOGroup *group = vcdev->vdev.group;
>
> vfio_ccw_unregister_irq_notifier(vcdev, VFIO_CCW_REQ_IRQ_INDEX);
> vfio_ccw_unregister_irq_notifier(vcdev, VFIO_CCW_CRW_IRQ_INDEX);
> vfio_ccw_unregister_irq_notifier(vcdev, VFIO_CCW_IO_IRQ_INDEX);
> vfio_ccw_put_region(vcdev);
> - vfio_ccw_put_device(vcdev);
> - vfio_put_group(group);
> + vfio_detach_device(&vcdev->vdev);
> + g_free(vcdev->vdev.name);
>
> if (cdc->unrealize) {
> cdc->unrealize(cdev);
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 74556da0c7..c71fddc09a 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -837,7 +837,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
> }
> }
>
> -VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp)
> +static VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp)
> {
> VFIOGroup *group;
> char path[32];
> @@ -900,7 +900,7 @@ free_group_exit:
> return NULL;
> }
>
> -void vfio_put_group(VFIOGroup *group)
> +static void vfio_put_group(VFIOGroup *group)
> {
> if (!group || !QLIST_EMPTY(&group->device_list)) {
> return;
> @@ -917,8 +917,8 @@ void vfio_put_group(VFIOGroup *group)
> g_free(group);
> }
>
> -int vfio_get_device(VFIOGroup *group, const char *name,
> - VFIODevice *vbasedev, Error **errp)
> +static int vfio_get_device(VFIOGroup *group, const char *name,
> + VFIODevice *vbasedev, Error **errp)
> {
> g_autofree struct vfio_device_info *info = NULL;
> int fd;
> @@ -976,7 +976,7 @@ int vfio_get_device(VFIOGroup *group, const char *name,
> return 0;
> }
>
> -void vfio_put_base_device(VFIODevice *vbasedev)
> +static void vfio_put_base_device(VFIODevice *vbasedev)
> {
> if (!vbasedev->group) {
> return;
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index a29dfe7723..95bcafdaf6 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -230,7 +230,6 @@ int vfio_container_add_section_window(VFIOContainer *container,
> void vfio_container_del_section_window(VFIOContainer *container,
> MemoryRegionSection *section);
>
> -void vfio_put_base_device(VFIODevice *vbasedev);
> void vfio_disable_irqindex(VFIODevice *vbasedev, int index);
> void vfio_unmask_single_irqindex(VFIODevice *vbasedev, int index);
> void vfio_mask_single_irqindex(VFIODevice *vbasedev, int index);
> @@ -248,11 +247,7 @@ void vfio_region_unmap(VFIORegion *region);
> void vfio_region_exit(VFIORegion *region);
> void vfio_region_finalize(VFIORegion *region);
> void vfio_reset_handler(void *opaque);
> -VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp);
> -void vfio_put_group(VFIOGroup *group);
> struct vfio_device_info *vfio_get_device_info(int fd);
> -int vfio_get_device(VFIOGroup *group, const char *name,
> - VFIODevice *vbasedev, Error **errp);
> int vfio_attach_device(char *name, VFIODevice *vbasedev,
> AddressSpace *as, Error **errp);
> void vfio_detach_device(VFIODevice *vbasedev);
>-----Original Message----- >From: Cédric Le Goater <clg@redhat.com> >Sent: Thursday, September 21, 2023 8:20 PM >Subject: Re: [PATCH v1 12/22] vfio/ccw: Use vfio_[attach/detach]_device > >On 8/30/23 12:37, Zhenzhong Duan wrote: >> From: Eric Auger <eric.auger@redhat.com> >> >> Let the vfio-ccw device use vfio_attach_device() and >> vfio_detach_device(), hence hiding the details of the used >> IOMMU backend. >> >> Also now all the devices have been migrated to use the new >> vfio_attach_device/vfio_detach_device API, let's turn the >> legacy functions into static functions, local to container.c. >> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> Signed-off-by: Yi Liu <yi.l.liu@intel.com> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > >Zhenzhong, > >Could you please resend 1-12 independantly as a prereq series for iommufd >support ? I think there wouldn't be much to say and they could be merged >pretty quickly. Got it, will do. Note I want to replace "[PATCH v1 06/22] vfio/common: Add a vfio device iterator" with vfio_device_list which will be used by both BEs, that may need some time. Thanks Zhenzhong
On 9/21/23 15:00, Duan, Zhenzhong wrote: > > >> -----Original Message----- >> From: Cédric Le Goater <clg@redhat.com> >> Sent: Thursday, September 21, 2023 8:20 PM >> Subject: Re: [PATCH v1 12/22] vfio/ccw: Use vfio_[attach/detach]_device >> >> On 8/30/23 12:37, Zhenzhong Duan wrote: >>> From: Eric Auger <eric.auger@redhat.com> >>> >>> Let the vfio-ccw device use vfio_attach_device() and >>> vfio_detach_device(), hence hiding the details of the used >>> IOMMU backend. >>> >>> Also now all the devices have been migrated to use the new >>> vfio_attach_device/vfio_detach_device API, let's turn the >>> legacy functions into static functions, local to container.c. >>> >>> Signed-off-by: Eric Auger <eric.auger@redhat.com> >>> Signed-off-by: Yi Liu <yi.l.liu@intel.com> >>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >> >> Zhenzhong, >> >> Could you please resend 1-12 independantly as a prereq series for iommufd >> support ? I think there wouldn't be much to say and they could be merged >> pretty quickly. > > Got it, will do. > Note I want to replace "[PATCH v1 06/22] vfio/common: Add a vfio device iterator" > with vfio_device_list which will be used by both BEs, that may need some time. Sure. Not today :) When you can. Thanks, C.
© 2016 - 2026 Red Hat, Inc.