On 9/23/25 04:45, Duan, Zhenzhong wrote:


-----Original Message-----
From: Cédric Le Goater <[email protected]>
Subject: Re: [PATCH 1/5] vfio/iommufd: Add framework code to support
getting dirty bitmap before unmap

On 9/22/25 05:17, Duan, Zhenzhong wrote:
Hi Cedric,

-----Original Message-----
From: Cédric Le Goater <[email protected]>
Subject: Re: [PATCH 1/5] vfio/iommufd: Add framework code to support
getting dirty bitmap before unmap

Hello Zhenzhong

On 9/10/25 04:36, Zhenzhong Duan wrote:
Currently we support device and iommu dirty tracking, device dirty
tracking is preferred.

Add the framework code in iommufd_cdev_unmap_one() to choose
either
device or iommu dirty tracking, just like vfio_legacy_dma_unmap_one().

I wonder if commit 567d7d3e6be5 ("vfio/common: Work around kernel
overflow bug in DMA unmap") could be removed now to make the code
common to both VFIO IOMMU Type1 and IOMMUFD backends.

I am not clear if there is other reason to keep the workaround, but the
original
kernel issue had been fixed with below commit:

commit 58fec830fc19208354895d9832785505046d6c01
Author: Alex Williamson <[email protected]>
Date:   Mon Jan 7 22:13:22 2019 -0700

      vfio/type1: Fix unmap overflow off-by-one

      The below referenced commit adds a test for integer overflow, but in
      doing so prevents the unmap ioctl from ever including the last page
of
      the address space.  Subtract one to compare to the last address of
the
      unmap to avoid the overflow and wrap-around.

      Fixes: 71a7d3d78e3c ("vfio/type1: silence integer overflow warning")
      Link: https://bugzilla.redhat.com/show_bug.cgi?id=1662291
      Cc: [email protected] # v4.15+
      Reported-by: Pei Zhang <[email protected]>
      Debugged-by: Peter Xu <[email protected]>
      Reviewed-by: Dan Carpenter <[email protected]>
      Reviewed-by: Peter Xu <[email protected]>
      Tested-by: Peter Xu <[email protected]>
      Reviewed-by: Cornelia Huck <[email protected]>
      Signed-off-by: Alex Williamson <[email protected]>


I asked Alex and Peter in another thread.

Just curious on the answer, may I ask which thread?

According to Alex, the QEMU workaround can be removed :

https://lore.kernel.org/qemu-devel/20250919102447.748e17fe.alex.williams
[email protected]/

btw: I just found unmapping in halves seems unnecessary as both backends
of kernel side support unmap_all now.

      if (unmap_all) {
          /* The unmap ioctl doesn't accept a full 64-bit span. */
          Int128 llsize = int128_rshift(int128_2_64(), 1);

          ret = vfio_legacy_dma_unmap_one(bcontainer, 0,
int128_get64(llsize),
                                          iotlb);

          if (ret == 0) {
              ret = vfio_legacy_dma_unmap_one(bcontainer,
int128_get64(llsize),
                                              int128_get64(llsize),
iotlb);
          }

      } else {
          ret = vfio_legacy_dma_unmap_one(bcontainer, iova, size,
iotlb);
      }

Good. So we can simply both backends it seems.

*ify


Will you handle them or not? I mean the workaround removing and unmapping_all 
optimization.

I can revert 567d7d3e6be5 ("vfio/common: Work around kernel overflow
bug in DMA unmap") but, AFAICT, the "unmap DMAs in halves" method (see
1b296c3def4b) should be kept.

Thanks,

C.


Reply via email to