Re: [PATCH RFC 07/12] iommufd: Data structure to provide IOVA to PFN mapping

From: Yi Liu <yi.l.liu@intel.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Alex Williamson <alex.williamson@redhat.com>,
	Lu Baolu <baolu.lu@linux.intel.com>,
	Chaitanya Kulkarni <chaitanyak@nvidia.com>,
	Cornelia Huck <cohuck@redhat.com>,
	Daniel Jordan <daniel.m.jordan@oracle.com>,
	David Gibson <david@gibson.dropbear.id.au>,
	Eric Auger <eric.auger@redhat.com>,
	<iommu@lists.linux-foundation.org>,
	"Jason Wang" <jasowang@redhat.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.org>,
	Joao Martins <joao.m.martins@oracle.com>,
	Kevin Tian <kevin.tian@intel.com>, <kvm@vger.kernel.org>,
	Matthew Rosato <mjrosato@linux.ibm.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Nicolin Chen <nicolinc@nvidia.com>,
	"Niklas Schnelle" <schnelle@linux.ibm.com>,
	Shameerali Kolothum Thodi  <shameerali.kolothum.thodi@huawei.com>,
	Keqian Zhu <zhukeqian1@huawei.com>
Subject: Re: [PATCH RFC 07/12] iommufd: Data structure to provide IOVA to PFN mapping
Date: Mon, 18 Apr 2022 18:47:33 +0800	[thread overview]
Message-ID: <b5916063-5c16-b9d0-2825-7b6f858ee3fe@intel.com> (raw)
In-Reply-To: <dea7b099-cc21-51f0-e674-50901a7a966b@intel.com>

Hi Jason,

On 2022/4/17 22:56, Yi Liu wrote:
> On 2022/4/13 22:49, Yi Liu wrote:
>> On 2022/4/13 22:36, Jason Gunthorpe wrote:
>>> On Wed, Apr 13, 2022 at 10:02:58PM +0800, Yi Liu wrote:
>>>>> +/**
>>>>> + * iopt_unmap_iova() - Remove a range of iova
>>>>> + * @iopt: io_pagetable to act on
>>>>> + * @iova: Starting iova to unmap
>>>>> + * @length: Number of bytes to unmap
>>>>> + *
>>>>> + * The requested range must exactly match an existing range.
>>>>> + * Splitting/truncating IOVA mappings is not allowed.
>>>>> + */
>>>>> +int iopt_unmap_iova(struct io_pagetable *iopt, unsigned long iova,
>>>>> +            unsigned long length)
>>>>> +{
>>>>> +    struct iopt_pages *pages;
>>>>> +    struct iopt_area *area;
>>>>> +    unsigned long iova_end;
>>>>> +    int rc;
>>>>> +
>>>>> +    if (!length)
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    if (check_add_overflow(iova, length - 1, &iova_end))
>>>>> +        return -EOVERFLOW;
>>>>> +
>>>>> +    down_read(&iopt->domains_rwsem);
>>>>> +    down_write(&iopt->iova_rwsem);
>>>>> +    area = iopt_find_exact_area(iopt, iova, iova_end);
>>>>
>>>> when testing vIOMMU with Qemu using iommufd, I hit a problem as log #3
>>>> shows. Qemu failed when trying to do map due to an IOVA still in use.
>>>> After debugging, the 0xfffff000 IOVA is mapped but not unmapped. But 
>>>> per log
>>>> #2, Qemu has issued unmap with a larger range (0xff000000 -
>>>> 0x100000000) which includes the 0xfffff000. But iopt_find_exact_area()
>>>> doesn't find any area. So 0xfffff000 is not unmapped. Is this correct? 
>>>> Same
>>>> test passed with vfio iommu type1 driver. any idea?
>>>
>>> There are a couple of good reasons why the iopt_unmap_iova() should
>>> proccess any contiguous range of fully contained areas, so I would
>>> consider this something worth fixing. can you send a small patch and
>>> test case and I'll fold it in?
>>
>> sure. just spotted it, so haven't got fix patch yet. I may work on
>> it tomorrow.
> 
> Hi Jason,
> 
> Got below patch for it. Also pushed to the exploration branch.
> 
> https://github.com/luxis1999/iommufd/commit/d764f3288de0fd52c578684788a437701ec31b2d 

0-day reports a use without initialization to me. So updated it. Please get
the change in below commit. Sorry for the noise.

https://github.com/luxis1999/iommufd/commit/10674417c235cb4a4caf2202fffb078611441da2

-- 
Regards,
Yi Liu