From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4CB3FC433EF for ; Wed, 20 Jul 2022 19:41:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233561AbiGTTlV (ORCPT ); Wed, 20 Jul 2022 15:41:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46594 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229570AbiGTTlU (ORCPT ); Wed, 20 Jul 2022 15:41:20 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 5E2E25143E for ; Wed, 20 Jul 2022 12:41:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1658346078; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ehh4V90wN0RJR9qCy+oaecsD0c4O/PC+hN+Ir98d0Ho=; b=gGSIfsfn1+yyekmGIyBu7EyoTKi/3qKa5NmRRF1/6fnGn2ZVJhjlvJd1jloH6yaE1uwrKq oU3grPZjpIPlKXGJ06EDFkliVNk3mF+fwIxW5eTtEZB/U2ZVNbLcQ+wU5QzVVyRbKJHy51 KqHqXg4tM8i5kYZ4nAhMpto5QDluiyQ= Received: from mail-il1-f199.google.com (mail-il1-f199.google.com [209.85.166.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-580-dkSppk0UPza196aCoAxGaw-1; Wed, 20 Jul 2022 15:41:17 -0400 X-MC-Unique: dkSppk0UPza196aCoAxGaw-1 Received: by mail-il1-f199.google.com with SMTP id f2-20020a056e020b4200b002dca33f6243so11864851ilu.15 for ; Wed, 20 Jul 2022 12:41:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:organization:mime-version:content-transfer-encoding; bh=ehh4V90wN0RJR9qCy+oaecsD0c4O/PC+hN+Ir98d0Ho=; b=69zIS9TwYcrKvGSYWkJDo1+628kpNbKDVnIZDWB4VFChmtYicEw3O5G2WyVeJDPpKo exkFGC3zoN8Y2jhEIoslfXlyZmLJZf/56vPjIaZWAqkXXlErL9zzXvw5bHA5GKYthbcV rsrZDNsHBSXiohuhYeJYskfKmHWSSZaA/6um5q4KGpVwHz3nfUnu8+bKBAP6Cj/OCZEa X+iKsiLzArWWGYyKe6MXhWJdpN89hR8eyzWr5vX0asnY9Aan6GHTsiisCQ/K6Jzq6uaV 4QH+PCp2wi7bItEkZXU091fYIV6v6fek5wmfgC036PUBqKdOxz+Yu8cg97BNkiRTCVWF 1SYQ== X-Gm-Message-State: AJIora9Q9mkVkbRrGfovwynYky46GHaQG/va5prmWyDCJSnDEiWoAPGZ e0URTgg5e0b3SGPc8iAADma9LJeAZBrLhsddcghVyKCk7skOsoY6ZlI6fLpQve/tyZaU4SsxdGB yMMLQ6fBUirPH9OVMRMLKug== X-Received: by 2002:a05:6e02:1a0c:b0:2dc:8921:a8d9 with SMTP id s12-20020a056e021a0c00b002dc8921a8d9mr20755563ild.145.1658346076499; Wed, 20 Jul 2022 12:41:16 -0700 (PDT) X-Google-Smtp-Source: AGRyM1vYxHlGrytgU3tCmIJ+Uhn7Hc+/RyJhZDLkKiiqtiSiv/P5YPbnk+rkd8QUKXd1S1jL+Xv4Pg== X-Received: by 2002:a05:6e02:1a0c:b0:2dc:8921:a8d9 with SMTP id s12-20020a056e021a0c00b002dc8921a8d9mr20755544ild.145.1658346076213; Wed, 20 Jul 2022 12:41:16 -0700 (PDT) Received: from redhat.com ([38.15.36.239]) by smtp.gmail.com with ESMTPSA id g40-20020a022728000000b00339eedc7840sm8267773jaa.94.2022.07.20.12.41.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Jul 2022 12:41:15 -0700 (PDT) Date: Wed, 20 Jul 2022 13:41:13 -0600 From: Alex Williamson To: Jason Gunthorpe Cc: Alexander Gordeev , David Airlie , Christian Borntraeger , Cornelia Huck , Daniel Vetter , dri-devel@lists.freedesktop.org, Harald Freudenberger , Vasily Gorbik , Heiko Carstens , intel-gfx@lists.freedesktop.org, intel-gvt-dev@lists.freedesktop.org, Jani Nikula , Jason Herne , Joonas Lahtinen , kvm@vger.kernel.org, linux-s390@vger.kernel.org, Matthew Rosato , Peter Oberparleiter , Halil Pasic , Rodrigo Vivi , Sven Schnelle , Tvrtko Ursulin , Vineeth Vijayan , Zhi Wang , Tony Krowiak , Eric Farman , Christoph Hellwig , Kevin Tian , Zhenyu Wang , Nicolin Chen Subject: Re: [PATCH v4 1/2] vfio: Replace the DMA unmapping notifier with a callback Message-ID: <20220720134113.4225f9d6.alex.williamson@redhat.com> In-Reply-To: <1-v4-681e038e30fd+78-vfio_unmap_notif_jgg@nvidia.com> References: <0-v4-681e038e30fd+78-vfio_unmap_notif_jgg@nvidia.com> <1-v4-681e038e30fd+78-vfio_unmap_notif_jgg@nvidia.com> Organization: Red Hat MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-s390@vger.kernel.org On Tue, 19 Jul 2022 21:02:48 -0300 Jason Gunthorpe wrote: > diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c > index a7d2a95796d360..bb1a1677c5c230 100644 > --- a/drivers/s390/crypto/vfio_ap_ops.c > +++ b/drivers/s390/crypto/vfio_ap_ops.c > @@ -1226,34 +1226,14 @@ static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev, > return 0; > } > > -/** > - * vfio_ap_mdev_iommu_notifier - IOMMU notifier callback > - * > - * @nb: The notifier block > - * @action: Action to be taken > - * @data: data associated with the request > - * > - * For an UNMAP request, unpin the guest IOVA (the NIB guest address we > - * pinned before). Other requests are ignored. > - * > - * Return: for an UNMAP request, NOFITY_OK; otherwise NOTIFY_DONE. > - */ > -static int vfio_ap_mdev_iommu_notifier(struct notifier_block *nb, > - unsigned long action, void *data) > +static void vfio_ap_mdev_dma_unmap(struct vfio_device *vdev, u64 iova, > + u64 length) > { > - struct ap_matrix_mdev *matrix_mdev; > - > - matrix_mdev = container_of(nb, struct ap_matrix_mdev, iommu_notifier); > - > - if (action == VFIO_IOMMU_NOTIFY_DMA_UNMAP) { > - struct vfio_iommu_type1_dma_unmap *unmap = data; > - unsigned long g_pfn = unmap->iova >> PAGE_SHIFT; > - > - vfio_unpin_pages(&matrix_mdev->vdev, &g_pfn, 1); > - return NOTIFY_OK; > - } > + struct ap_matrix_mdev *matrix_mdev = > + container_of(vdev, struct ap_matrix_mdev, vdev); > + unsigned long g_pfn = iova >> PAGE_SHIFT; > > - return NOTIFY_DONE; > + vfio_unpin_pages(&matrix_mdev->vdev, &g_pfn, 1); > } > > /** I tried to apply this on top of Nicolin's series which results in a conflict that can be resolved as below: diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index e8856a7e151c..d7c38c82f694 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1219,33 +1219,13 @@ static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev, return 0; } -/** - * vfio_ap_mdev_iommu_notifier - IOMMU notifier callback - * - * @nb: The notifier block - * @action: Action to be taken - * @data: data associated with the request - * - * For an UNMAP request, unpin the guest IOVA (the NIB guest address we - * pinned before). Other requests are ignored. - * - * Return: for an UNMAP request, NOFITY_OK; otherwise NOTIFY_DONE. - */ -static int vfio_ap_mdev_iommu_notifier(struct notifier_block *nb, - unsigned long action, void *data) +static void vfio_ap_mdev_dma_unmap(struct vfio_device *vdev, u64 iova, + u64 length) { - struct ap_matrix_mdev *matrix_mdev; - - matrix_mdev = container_of(nb, struct ap_matrix_mdev, iommu_notifier); - - if (action == VFIO_IOMMU_NOTIFY_DMA_UNMAP) { - struct vfio_iommu_type1_dma_unmap *unmap = data; - - vfio_unpin_pages(&matrix_mdev->vdev, unmap->iova, 1); - return NOTIFY_OK; - } + struct ap_matrix_mdev *matrix_mdev = + container_of(vdev, struct ap_matrix_mdev, vdev); - return NOTIFY_DONE; + vfio_unpin_pages(&matrix_mdev->vdev, iova, 1); } /** ie. we don't need the gfn, we only need the iova. However then I start to wonder why we're passing in 1 for the number of pages because this previously notifier, now callback is called for the entire vfio_dma range when we find any pinned pages. It makes no sense for a driver to assume that the first iova is pinned and is the only pinned page. ccw has the same issue: static void vfio_ccw_dma_unmap(struct vfio_device *vdev, u64 iova, u64 length) { struct vfio_ccw_private *private = container_of(vdev, struct vfio_ccw_private, vdev); /* Drivers MUST unpin pages in response to an invalidation. */ if (!cp_iova_pinned(&private->cp, iova)) return; vfio_ccw_mdev_reset(private); } Entirely ignoring the length arg. It seems only GVT-g has this correct to actually look through the extent of the range being unmapped: static void intel_vgpu_dma_unmap(struct vfio_device *vfio_dev, u64 iova, u64 length) { struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev); struct gvt_dma *entry; u64 iov_pfn = iova >> PAGE_SHIFT; u64 end_iov_pfn = iov_pfn + length / PAGE_SIZE; mutex_lock(&vgpu->cache_lock); for (; iov_pfn < end_iov_pfn; iov_pfn++) { entry = __gvt_cache_find_gfn(vgpu, iov_pfn); if (!entry) continue; gvt_dma_unmap_page(vgpu, entry->gfn, entry->dma_addr, entry->size); __gvt_cache_remove_entry(vgpu, entry); } mutex_unlock(&vgpu->cache_lock); } Should ap and ccw implementations of .dma_unmap just be replaced with a BUG_ON(1)? Thanks, Alex