On Fri, 15 Jan 2021 17:26:43 +0800
Keqian Zhu <zhukeqian1@xxxxxxxxxx> wrote:
> vfio_sanity_check_pfn_list() is used to check whether pfn_list of
> vfio_dma is empty when remove the external domain, so it makes a
> wrong assumption that only external domain will add pfn to dma pfn_list.
>
> Now we apply this check when remove a specific vfio_dma and extract
> the notifier check just for external domain.
The page pinning interface is gated by having a notifier registered for
unmaps, therefore non-external domains would also need to register a
notifier. There's currently no other way to add entries to the
pfn_list. So if we allow pinning for such domains, then it's wrong to
WARN_ON() when the notifier list is not-empty when removing an external
domain. Long term we should probably extend page {un}pinning for the
caller to pass their notifier to be validated against the notifier list
rather than just allowing page pinning if *any* notifier is registered.
Thanks,
Alex
> Fixes: a54eb55045ae ("vfio iommu type1: Add support for mediated devices")
> Signed-off-by: Keqian Zhu <zhukeqian1@xxxxxxxxxx>
> ---
> drivers/vfio/vfio_iommu_type1.c | 24 +++++-------------------
> 1 file changed, 5 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 4e82b9a3440f..a9bc15e84a4e 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -958,6 +958,7 @@ static long vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma,
>
> static void vfio_remove_dma(struct vfio_iommu *iommu, struct vfio_dma *dma)
> {
> + WARN_ON(!RB_EMPTY_ROOT(&dma->pfn_list);
> vfio_unmap_unpin(iommu, dma, true);
> vfio_unlink_dma(iommu, dma);
> put_task_struct(dma->task);
> @@ -2251,23 +2252,6 @@ static void vfio_iommu_unmap_unpin_reaccount(struct vfio_iommu *iommu)
> }
> }
>
> -static void vfio_sanity_check_pfn_list(struct vfio_iommu *iommu)
> -{
> - struct rb_node *n;
> -
> - n = rb_first(&iommu->dma_list);
> - for (; n; n = rb_next(n)) {
> - struct vfio_dma *dma;
> -
> - dma = rb_entry(n, struct vfio_dma, node);
> -
> - if (WARN_ON(!RB_EMPTY_ROOT(&dma->pfn_list)))
> - break;
> - }
> - /* mdev vendor driver must unregister notifier */
> - WARN_ON(iommu->notifier.head);
> -}
> -
> /*
> * Called when a domain is removed in detach. It is possible that
> * the removed domain decided the iova aperture window. Modify the
> @@ -2367,7 +2351,8 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
> kfree(group);
>
> if (list_empty(&iommu->external_domain->group_list)) {
> - vfio_sanity_check_pfn_list(iommu);
> + /* mdev vendor driver must unregister notifier */
> + WARN_ON(iommu->notifier.head);
>
> if (!IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu))
> vfio_iommu_unmap_unpin_all(iommu);
> @@ -2491,7 +2476,8 @@ static void vfio_iommu_type1_release(void *iommu_data)
>
> if (iommu->external_domain) {
> vfio_release_domain(iommu->external_domain, true);
> - vfio_sanity_check_pfn_list(iommu);
> + /* mdev vendor driver must unregister notifier */
> + WARN_ON(iommu->notifier.head);
> kfree(iommu->external_domain);
> }
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
