From: Eric Auger <eric.auger@redhat.com>
To: Zhenzhong Duan <zhenzhong.duan@intel.com>, qemu-devel@nongnu.org
Cc: alex.williamson@redhat.com, clg@redhat.com, jgg@nvidia.com,
nicolinc@nvidia.com, joao.m.martins@oracle.com,
peterx@redhat.com, jasowang@redhat.com, kevin.tian@intel.com,
yi.l.liu@intel.com, yi.y.sun@intel.com, chao.p.peng@intel.com
Subject: Re: [PATCH v2 11/12] vfio/common: Introduce two kinds of VFIO device lists
Date: Wed, 27 Sep 2023 15:13:37 +0200 [thread overview]
Message-ID: <dba250f5-4d75-b4d4-953b-6495f7ddbaf1@redhat.com> (raw)
In-Reply-To: <20230926113255.1177834-12-zhenzhong.duan@intel.com>
Hi Zhenzhong,
On 9/26/23 13:32, Zhenzhong Duan wrote:
> In VFIO subsystem, there are different VFIO device iteration requirements.
> One requirement is to iterate all VFIO devices, the other is to iterate
> VFIO device in same container.
>
> Currently VFIO device is iterated through VFIO group list which is group
> perceivable and less efficient.
>
> Introduce two kinds of VFIO device lists, one is a global list, the other
> is per container list. With the two lists added, we can make some migration
> and reset related functions group agnostic.
>
> For example, vfio_device_list is used in below functions:
> vfio_mig_active
> vfio_reset_handler
> vfio_multiple_devices_migration_is_supported
>
> Per container list is used in below functions:
> vfio_devices_all_dirty_tracking
> vfio_devices_all_device_dirty_tracking
> vfio_devices_all_running_and_mig_active
> vfio_devices_dma_logging_stop
> vfio_devices_dma_logging_start
> vfio_devices_query_dirty_bitmap
>
> This is a prerequisite for future IOMMUFD backend support which
> has same kind of iteration requirement.
>
> vfio_group_list is preserved for some functions which honor group
> iteration, those functions are all related to legacy backend.
>
> Suggested-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
This may be split into 3 patches
1. creation of container->device_list
2. creation of global device list
3. addition of container field in vbasedev (which is not described in
the commit msg by the way) and looks somehow unrelated to me?
Eric
> ---
> include/hw/vfio/vfio-common.h | 5 +
> hw/vfio/common.c | 194 ++++++++++++++++------------------
> 2 files changed, 94 insertions(+), 105 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index c486bdef2a..54905b9dd4 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -98,6 +98,7 @@ typedef struct VFIOContainer {
> QLIST_HEAD(, VFIOGroup) group_list;
> QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
> QLIST_ENTRY(VFIOContainer) next;
> + QLIST_HEAD(, VFIODevice) device_list;
> } VFIOContainer;
>
> typedef struct VFIOGuestIOMMU {
> @@ -129,7 +130,10 @@ typedef struct VFIODeviceOps VFIODeviceOps;
>
> typedef struct VFIODevice {
> QLIST_ENTRY(VFIODevice) next;
> + QLIST_ENTRY(VFIODevice) container_next;
> + QLIST_ENTRY(VFIODevice) global_next;
> struct VFIOGroup *group;
> + VFIOContainer *container;
> char *sysfsdev;
> char *name;
> DeviceState *dev;
> @@ -229,6 +233,7 @@ int vfio_kvm_device_del_fd(int fd, Error **errp);
>
> extern const MemoryRegionOps vfio_region_ops;
> typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList;
> +typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
> extern VFIOGroupList vfio_group_list;
>
> bool vfio_mig_active(void);
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 12ebf2f11d..645e2dc39a 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -48,6 +48,8 @@
>
> VFIOGroupList vfio_group_list =
> QLIST_HEAD_INITIALIZER(vfio_group_list);
> +static VFIODeviceList vfio_device_list =
> + QLIST_HEAD_INITIALIZER(vfio_device_list);
> static QLIST_HEAD(, VFIOAddressSpace) vfio_address_spaces =
> QLIST_HEAD_INITIALIZER(vfio_address_spaces);
>
> @@ -94,18 +96,15 @@ static int vfio_get_dirty_bitmap(VFIOContainer *container, uint64_t iova,
>
> bool vfio_mig_active(void)
> {
> - VFIOGroup *group;
> VFIODevice *vbasedev;
>
> - if (QLIST_EMPTY(&vfio_group_list)) {
> + if (QLIST_EMPTY(&vfio_device_list)) {
> return false;
> }
>
> - QLIST_FOREACH(group, &vfio_group_list, next) {
> - QLIST_FOREACH(vbasedev, &group->device_list, next) {
> - if (vbasedev->migration_blocker) {
> - return false;
> - }
> + QLIST_FOREACH(vbasedev, &vfio_device_list, next) {
> + if (vbasedev->migration_blocker) {
> + return false;
> }
> }
> return true;
> @@ -120,19 +119,16 @@ static Error *multiple_devices_migration_blocker;
> */
> static bool vfio_multiple_devices_migration_is_supported(void)
> {
> - VFIOGroup *group;
> VFIODevice *vbasedev;
> unsigned int device_num = 0;
> bool all_support_p2p = true;
>
> - QLIST_FOREACH(group, &vfio_group_list, next) {
> - QLIST_FOREACH(vbasedev, &group->device_list, next) {
> - if (vbasedev->migration) {
> - device_num++;
> + QLIST_FOREACH(vbasedev, &vfio_device_list, next) {
> + if (vbasedev->migration) {
> + device_num++;
>
> - if (!(vbasedev->migration->mig_flags & VFIO_MIGRATION_P2P)) {
> - all_support_p2p = false;
> - }
> + if (!(vbasedev->migration->mig_flags & VFIO_MIGRATION_P2P)) {
> + all_support_p2p = false;
> }
> }
> }
> @@ -184,7 +180,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)
> @@ -218,7 +214,6 @@ bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
>
> static bool vfio_devices_all_dirty_tracking(VFIOContainer *container)
> {
> - VFIOGroup *group;
> VFIODevice *vbasedev;
> MigrationState *ms = migrate_get_current();
>
> @@ -227,19 +222,17 @@ static bool vfio_devices_all_dirty_tracking(VFIOContainer *container)
> return false;
> }
>
> - QLIST_FOREACH(group, &container->group_list, container_next) {
> - QLIST_FOREACH(vbasedev, &group->device_list, next) {
> - VFIOMigration *migration = vbasedev->migration;
> + QLIST_FOREACH(vbasedev, &container->device_list, container_next) {
> + VFIOMigration *migration = vbasedev->migration;
>
> - if (!migration) {
> - return false;
> - }
> + if (!migration) {
> + return false;
> + }
>
> - if (vbasedev->pre_copy_dirty_page_tracking == ON_OFF_AUTO_OFF &&
> - (vfio_device_state_is_running(vbasedev) ||
> - vfio_device_state_is_precopy(vbasedev))) {
> - return false;
> - }
> + if (vbasedev->pre_copy_dirty_page_tracking == ON_OFF_AUTO_OFF &&
> + (vfio_device_state_is_running(vbasedev) ||
> + vfio_device_state_is_precopy(vbasedev))) {
> + return false;
> }
> }
> return true;
> @@ -247,14 +240,11 @@ static bool vfio_devices_all_dirty_tracking(VFIOContainer *container)
>
> static bool vfio_devices_all_device_dirty_tracking(VFIOContainer *container)
> {
> - VFIOGroup *group;
> VFIODevice *vbasedev;
>
> - QLIST_FOREACH(group, &container->group_list, container_next) {
> - QLIST_FOREACH(vbasedev, &group->device_list, next) {
> - if (!vbasedev->dirty_pages_supported) {
> - return false;
> - }
> + QLIST_FOREACH(vbasedev, &container->device_list, container_next) {
> + if (!vbasedev->dirty_pages_supported) {
> + return false;
> }
> }
>
> @@ -267,27 +257,24 @@ static bool vfio_devices_all_device_dirty_tracking(VFIOContainer *container)
> */
> static bool vfio_devices_all_running_and_mig_active(VFIOContainer *container)
> {
> - VFIOGroup *group;
> VFIODevice *vbasedev;
>
> if (!migration_is_active(migrate_get_current())) {
> return false;
> }
>
> - QLIST_FOREACH(group, &container->group_list, container_next) {
> - QLIST_FOREACH(vbasedev, &group->device_list, next) {
> - VFIOMigration *migration = vbasedev->migration;
> + QLIST_FOREACH(vbasedev, &container->device_list, container_next) {
> + VFIOMigration *migration = vbasedev->migration;
>
> - if (!migration) {
> - return false;
> - }
> + if (!migration) {
> + return false;
> + }
>
> - if (vfio_device_state_is_running(vbasedev) ||
> - vfio_device_state_is_precopy(vbasedev)) {
> - continue;
> - } else {
> - return false;
> - }
> + if (vfio_device_state_is_running(vbasedev) ||
> + vfio_device_state_is_precopy(vbasedev)) {
> + continue;
> + } else {
> + return false;
> }
> }
> return true;
> @@ -1187,20 +1174,17 @@ static bool vfio_section_is_vfio_pci(MemoryRegionSection *section,
> {
> VFIOPCIDevice *pcidev;
> VFIODevice *vbasedev;
> - VFIOGroup *group;
> Object *owner;
>
> owner = memory_region_owner(section->mr);
>
> - QLIST_FOREACH(group, &container->group_list, container_next) {
> - QLIST_FOREACH(vbasedev, &group->device_list, next) {
> - if (vbasedev->type != VFIO_DEVICE_TYPE_PCI) {
> - continue;
> - }
> - pcidev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
> - if (OBJECT(pcidev) == owner) {
> - return true;
> - }
> + QLIST_FOREACH(vbasedev, &container->device_list, container_next) {
> + if (vbasedev->type != VFIO_DEVICE_TYPE_PCI) {
> + continue;
> + }
> + pcidev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
> + if (OBJECT(pcidev) == owner) {
> + return true;
> }
> }
>
> @@ -1296,24 +1280,21 @@ static void vfio_devices_dma_logging_stop(VFIOContainer *container)
> sizeof(uint64_t))] = {};
> struct vfio_device_feature *feature = (struct vfio_device_feature *)buf;
> VFIODevice *vbasedev;
> - VFIOGroup *group;
>
> feature->argsz = sizeof(buf);
> feature->flags = VFIO_DEVICE_FEATURE_SET |
> VFIO_DEVICE_FEATURE_DMA_LOGGING_STOP;
>
> - QLIST_FOREACH(group, &container->group_list, container_next) {
> - QLIST_FOREACH(vbasedev, &group->device_list, next) {
> - if (!vbasedev->dirty_tracking) {
> - continue;
> - }
> + QLIST_FOREACH(vbasedev, &container->device_list, container_next) {
> + if (!vbasedev->dirty_tracking) {
> + continue;
> + }
>
> - if (ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature)) {
> - warn_report("%s: Failed to stop DMA logging, err %d (%s)",
> - vbasedev->name, -errno, strerror(errno));
> - }
> - vbasedev->dirty_tracking = false;
> + if (ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature)) {
> + warn_report("%s: Failed to stop DMA logging, err %d (%s)",
> + vbasedev->name, -errno, strerror(errno));
> }
> + vbasedev->dirty_tracking = false;
> }
> }
>
> @@ -1396,7 +1377,6 @@ static int vfio_devices_dma_logging_start(VFIOContainer *container)
> struct vfio_device_feature *feature;
> VFIODirtyRanges ranges;
> VFIODevice *vbasedev;
> - VFIOGroup *group;
> int ret = 0;
>
> vfio_dirty_tracking_init(container, &ranges);
> @@ -1406,21 +1386,19 @@ static int vfio_devices_dma_logging_start(VFIOContainer *container)
> return -errno;
> }
>
> - QLIST_FOREACH(group, &container->group_list, container_next) {
> - QLIST_FOREACH(vbasedev, &group->device_list, next) {
> - if (vbasedev->dirty_tracking) {
> - continue;
> - }
> + QLIST_FOREACH(vbasedev, &container->device_list, container_next) {
> + if (vbasedev->dirty_tracking) {
> + continue;
> + }
>
> - ret = ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature);
> - if (ret) {
> - ret = -errno;
> - error_report("%s: Failed to start DMA logging, err %d (%s)",
> - vbasedev->name, ret, strerror(errno));
> - goto out;
> - }
> - vbasedev->dirty_tracking = true;
> + ret = ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature);
> + if (ret) {
> + ret = -errno;
> + error_report("%s: Failed to start DMA logging, err %d (%s)",
> + vbasedev->name, ret, strerror(errno));
> + goto out;
> }
> + vbasedev->dirty_tracking = true;
> }
>
> out:
> @@ -1500,21 +1478,18 @@ static int vfio_devices_query_dirty_bitmap(VFIOContainer *container,
> hwaddr size)
> {
> VFIODevice *vbasedev;
> - VFIOGroup *group;
> int ret;
>
> - QLIST_FOREACH(group, &container->group_list, container_next) {
> - QLIST_FOREACH(vbasedev, &group->device_list, next) {
> - ret = vfio_device_dma_logging_report(vbasedev, iova, size,
> - vbmap->bitmap);
> - if (ret) {
> - error_report("%s: Failed to get DMA logging report, iova: "
> - "0x%" HWADDR_PRIx ", size: 0x%" HWADDR_PRIx
> - ", err: %d (%s)",
> - vbasedev->name, iova, size, ret, strerror(-ret));
> + QLIST_FOREACH(vbasedev, &container->device_list, container_next) {
> + ret = vfio_device_dma_logging_report(vbasedev, iova, size,
> + vbmap->bitmap);
> + if (ret) {
> + error_report("%s: Failed to get DMA logging report, iova: "
> + "0x%" HWADDR_PRIx ", size: 0x%" HWADDR_PRIx
> + ", err: %d (%s)",
> + vbasedev->name, iova, size, ret, strerror(-ret));
>
> - return ret;
> - }
> + return ret;
> }
> }
>
> @@ -1798,22 +1773,17 @@ bool vfio_get_info_dma_avail(struct vfio_iommu_type1_info *info,
>
> void vfio_reset_handler(void *opaque)
> {
> - VFIOGroup *group;
> VFIODevice *vbasedev;
>
> - QLIST_FOREACH(group, &vfio_group_list, next) {
> - QLIST_FOREACH(vbasedev, &group->device_list, next) {
> - if (vbasedev->dev->realized) {
> - vbasedev->ops->vfio_compute_needs_reset(vbasedev);
> - }
> + QLIST_FOREACH(vbasedev, &vfio_device_list, next) {
> + if (vbasedev->dev->realized) {
> + vbasedev->ops->vfio_compute_needs_reset(vbasedev);
> }
> }
>
> - QLIST_FOREACH(group, &vfio_group_list, next) {
> - QLIST_FOREACH(vbasedev, &group->device_list, next) {
> - if (vbasedev->dev->realized && vbasedev->needs_reset) {
> - vbasedev->ops->vfio_hot_reset_multi(vbasedev);
> - }
> + QLIST_FOREACH(vbasedev, &vfio_device_list, next) {
> + if (vbasedev->dev->realized && vbasedev->needs_reset) {
> + vbasedev->ops->vfio_hot_reset_multi(vbasedev);
> }
> }
> }
> @@ -2643,6 +2613,7 @@ int vfio_attach_device(char *name, VFIODevice *vbasedev,
> int groupid = vfio_device_groupid(vbasedev, errp);
> VFIODevice *vbasedev_iter;
> VFIOGroup *group;
> + VFIOContainer *container;
> int ret;
>
> if (groupid < 0) {
> @@ -2666,8 +2637,14 @@ int vfio_attach_device(char *name, VFIODevice *vbasedev,
> ret = vfio_get_device(group, name, vbasedev, errp);
> if (ret) {
> vfio_put_group(group);
> + return ret;
> }
>
> + container = group->container;
> + vbasedev->container = container;
> + QLIST_INSERT_HEAD(&container->device_list, vbasedev, container_next);
> + QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
> +
> return ret;
> }
>
> @@ -2675,6 +2652,13 @@ void vfio_detach_device(VFIODevice *vbasedev)
> {
> VFIOGroup *group = vbasedev->group;
>
> + if (!vbasedev->container) {
> + return;
> + }
> +
> + QLIST_REMOVE(vbasedev, global_next);
> + QLIST_REMOVE(vbasedev, container_next);
> + vbasedev->container = NULL;
> vfio_put_base_device(vbasedev);
> vfio_put_group(group);
> }
next prev parent reply other threads:[~2023-09-27 13:14 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-26 11:32 [PATCH v2 00/12] Prerequisite change for IOMMUFD support Zhenzhong Duan
2023-09-26 11:32 ` [PATCH v2 01/12] scripts/update-linux-headers: Add iommufd.h Zhenzhong Duan
2023-09-26 16:42 ` Cédric Le Goater
2023-09-26 11:32 ` [PATCH v2 02/12] linux-headers: " Zhenzhong Duan
2023-09-26 16:44 ` Cédric Le Goater
2023-09-26 11:32 ` [PATCH v2 03/12] vfio/common: Move IOMMU agnostic helpers to a separate file Zhenzhong Duan
2023-09-26 16:45 ` Cédric Le Goater
2023-09-26 11:32 ` [PATCH v2 04/12] vfio/common: Introduce vfio_container_add|del_section_window() Zhenzhong Duan
2023-09-27 10:12 ` Cédric Le Goater
2023-09-27 12:15 ` Eric Auger
2023-09-26 11:32 ` [PATCH v2 05/12] vfio/common: Extract out vfio_kvm_device_[add/del]_fd Zhenzhong Duan
2023-09-27 7:08 ` Eric Auger
2023-09-26 11:32 ` [PATCH v2 06/12] vfio/pci: Introduce vfio_[attach/detach]_device Zhenzhong Duan
2023-09-27 7:35 ` Eric Auger
2023-09-27 9:58 ` Duan, Zhenzhong
2023-09-27 10:02 ` Eric Auger
2023-09-27 9:01 ` Eric Auger
2023-09-27 10:07 ` Duan, Zhenzhong
2023-09-27 12:22 ` Eric Auger
2023-09-27 12:32 ` Duan, Zhenzhong
2023-09-27 12:38 ` Eric Auger
2023-10-02 13:57 ` Eric Auger
2023-09-27 10:16 ` Cédric Le Goater
2023-09-27 12:15 ` Duan, Zhenzhong
2023-09-26 11:32 ` [PATCH v2 07/12] vfio/platform: Use vfio_[attach/detach]_device Zhenzhong Duan
2023-09-27 9:10 ` Eric Auger
2023-09-27 10:11 ` Duan, Zhenzhong
2023-09-26 11:32 ` [PATCH v2 08/12] vfio/ap: " Zhenzhong Duan
2023-09-27 9:16 ` Eric Auger
2023-09-27 11:52 ` Duan, Zhenzhong
2023-09-27 12:24 ` Eric Auger
2023-09-27 12:30 ` Duan, Zhenzhong
2023-09-27 12:37 ` Eric Auger
2023-09-26 11:32 ` [PATCH v2 09/12] vfio/ccw: " Zhenzhong Duan
2023-09-27 10:00 ` Eric Auger
2023-09-27 12:09 ` Duan, Zhenzhong
2023-09-27 13:25 ` Matthew Rosato
2023-09-28 2:55 ` Duan, Zhenzhong
2023-09-26 11:32 ` [PATCH v2 10/12] vfio/common: Move VFIO reset handler registration to a group agnostic function Zhenzhong Duan
2023-09-26 11:32 ` [PATCH v2 11/12] vfio/common: Introduce two kinds of VFIO device lists Zhenzhong Duan
2023-09-27 13:13 ` Eric Auger [this message]
2023-09-28 2:38 ` Duan, Zhenzhong
2023-09-26 11:32 ` [PATCH v2 12/12] vfio/common: Move legacy VFIO backend code into separate container.c Zhenzhong Duan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dba250f5-4d75-b4d4-953b-6495f7ddbaf1@redhat.com \
--to=eric.auger@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=chao.p.peng@intel.com \
--cc=clg@redhat.com \
--cc=jasowang@redhat.com \
--cc=jgg@nvidia.com \
--cc=joao.m.martins@oracle.com \
--cc=kevin.tian@intel.com \
--cc=nicolinc@nvidia.com \
--cc=peterx@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=yi.l.liu@intel.com \
--cc=yi.y.sun@intel.com \
--cc=zhenzhong.duan@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).