[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BN9PR11MB5276FD77A2780C97BB82CBA88CA49@BN9PR11MB5276.namprd11.prod.outlook.com>
Date: Wed, 8 Jun 2022 08:35:47 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Nicolin Chen <nicolinc@...dia.com>,
"jgg@...dia.com" <jgg@...dia.com>,
"joro@...tes.org" <joro@...tes.org>,
"will@...nel.org" <will@...nel.org>,
"marcan@...can.st" <marcan@...can.st>,
"sven@...npeter.dev" <sven@...npeter.dev>,
"robin.murphy@....com" <robin.murphy@....com>,
"robdclark@...il.com" <robdclark@...il.com>,
"m.szyprowski@...sung.com" <m.szyprowski@...sung.com>,
"krzysztof.kozlowski@...aro.org" <krzysztof.kozlowski@...aro.org>,
"baolu.lu@...ux.intel.com" <baolu.lu@...ux.intel.com>,
"agross@...nel.org" <agross@...nel.org>,
"bjorn.andersson@...aro.org" <bjorn.andersson@...aro.org>,
"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
"heiko@...ech.de" <heiko@...ech.de>,
"orsonzhai@...il.com" <orsonzhai@...il.com>,
"baolin.wang7@...il.com" <baolin.wang7@...il.com>,
"zhang.lyra@...il.com" <zhang.lyra@...il.com>,
"wens@...e.org" <wens@...e.org>,
"jernej.skrabec@...il.com" <jernej.skrabec@...il.com>,
"samuel@...lland.org" <samuel@...lland.org>,
"jean-philippe@...aro.org" <jean-philippe@...aro.org>,
"alex.williamson@...hat.com" <alex.williamson@...hat.com>
CC: "virtualization@...ts.linux-foundation.org"
<virtualization@...ts.linux-foundation.org>,
"thierry.reding@...il.com" <thierry.reding@...il.com>,
"alim.akhtar@...sung.com" <alim.akhtar@...sung.com>,
"alyssa@...enzweig.io" <alyssa@...enzweig.io>,
"linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
"linux-samsung-soc@...r.kernel.org"
<linux-samsung-soc@...r.kernel.org>,
"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"jonathanh@...dia.com" <jonathanh@...dia.com>,
"linux-rockchip@...ts.infradead.org"
<linux-rockchip@...ts.infradead.org>,
"gerald.schaefer@...ux.ibm.com" <gerald.schaefer@...ux.ibm.com>,
"linux-sunxi@...ts.linux.dev" <linux-sunxi@...ts.linux.dev>,
"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
"linux-mediatek@...ts.infradead.org"
<linux-mediatek@...ts.infradead.org>,
"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"cohuck@...hat.com" <cohuck@...hat.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
"dwmw2@...radead.org" <dwmw2@...radead.org>
Subject: RE: [PATCH 4/5] vfio/iommu_type1: Clean up update_dirty_scope in
detach_group()
> From: Nicolin Chen
> Sent: Monday, June 6, 2022 2:19 PM
>
> All devices in emulated_iommu_groups have pinned_page_dirty_scope
> set, so the update_dirty_scope in the first list_for_each_entry
> is always false. Clean it up, and move the "if update_dirty_scope"
> part from the detach_group_done routine to the domain_list part.
>
> Rename the "detach_group_done" goto label accordingly.
>
> Suggested-by: Jason Gunthorpe <jgg@...dia.com>
> Signed-off-by: Nicolin Chen <nicolinc@...dia.com>
> ---
> drivers/vfio/vfio_iommu_type1.c | 27 ++++++++++++---------------
> 1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c
> b/drivers/vfio/vfio_iommu_type1.c
> index f4e3b423a453..b45b1cc118ef 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -2463,14 +2463,12 @@ static void
> vfio_iommu_type1_detach_group(void *iommu_data,
> struct vfio_iommu *iommu = iommu_data;
> struct vfio_domain *domain;
> struct vfio_iommu_group *group;
> - bool update_dirty_scope = false;
> LIST_HEAD(iova_copy);
>
> mutex_lock(&iommu->lock);
> list_for_each_entry(group, &iommu->emulated_iommu_groups,
> next) {
> if (group->iommu_group != iommu_group)
> continue;
> - update_dirty_scope = !group->pinned_page_dirty_scope;
> list_del(&group->next);
> kfree(group);
>
> @@ -2479,7 +2477,7 @@ static void vfio_iommu_type1_detach_group(void
> *iommu_data,
> WARN_ON(iommu->notifier.head);
> vfio_iommu_unmap_unpin_all(iommu);
> }
> - goto detach_group_done;
> + goto out_unlock;
> }
>
> /*
> @@ -2495,9 +2493,7 @@ static void vfio_iommu_type1_detach_group(void
> *iommu_data,
> continue;
>
> iommu_detach_group(domain->domain, group-
> >iommu_group);
> - update_dirty_scope = !group->pinned_page_dirty_scope;
> list_del(&group->next);
> - kfree(group);
> /*
> * Group ownership provides privilege, if the group list is
> * empty, the domain goes away. If it's the last domain with
> @@ -2519,7 +2515,17 @@ static void vfio_iommu_type1_detach_group(void
> *iommu_data,
> kfree(domain);
> vfio_iommu_aper_expand(iommu, &iova_copy);
> vfio_update_pgsize_bitmap(iommu);
> + /*
> + * Removal of a group without dirty tracking may
> allow
> + * the iommu scope to be promoted.
> + */
> + if (!group->pinned_page_dirty_scope) {
> + iommu->num_non_pinned_groups--;
> + if (iommu->dirty_page_tracking)
> +
> vfio_iommu_populate_bitmap_full(iommu);
This doesn't look correct. The old code decrements
num_non_pinned_groups for every detach group without dirty
tracking. But now it's only done when the domain is about to
be released...
> + }
> }
> + kfree(group);
> break;
> }
>
> @@ -2528,16 +2534,7 @@ static void vfio_iommu_type1_detach_group(void
> *iommu_data,
> else
> vfio_iommu_iova_free(&iova_copy);
>
> -detach_group_done:
> - /*
> - * Removal of a group without dirty tracking may allow the iommu
> scope
> - * to be promoted.
> - */
> - if (update_dirty_scope) {
> - iommu->num_non_pinned_groups--;
> - if (iommu->dirty_page_tracking)
> - vfio_iommu_populate_bitmap_full(iommu);
> - }
> +out_unlock:
> mutex_unlock(&iommu->lock);
> }
>
> --
> 2.17.1
>
> _______________________________________________
> iommu mailing list
> iommu@...ts.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
Powered by blists - more mailing lists