>-----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/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.
OK, let me take the unmap_all part. For example, for iommufd, it can be
simplified to:
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -67,19 +67,8 @@ static int iommufd_cdev_unmap(const VFIOContainerBase
*bcontainer,
/* unmap in halves */
if (unmap_all) {
- Int128 llsize = int128_rshift(int128_2_64(), 1);
- int ret;
-
- ret = iommufd_backend_unmap_dma(container->be, container->ioas_id,
- 0, int128_get64(llsize));
-
- if (ret == 0) {
- ret = iommufd_backend_unmap_dma(container->be, container->ioas_id,
- int128_get64(llsize),
- int128_get64(llsize));
- }
-
- return ret;
+ assert(!iova && !size);
+ size = UINT64_MAX;
}
/* TODO: Handle dma_unmap_bitmap with iotlb args (migration) */
Thanks
Zhenzhong
