qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v7 3/4] vfio iommu: Add support for mediated dev


From: Kirti Wankhede
Subject: Re: [Qemu-devel] [PATCH v7 3/4] vfio iommu: Add support for mediated devices
Date: Thu, 29 Sep 2016 20:36:30 +0530


On 9/29/2016 7:47 AM, Jike Song wrote:
> +Guangrong
> 
> On 08/25/2016 11:53 AM, Kirti Wankhede wrote:

...

>> +static long vfio_iommu_type1_pin_pages(void *iommu_data,
>> +                                   unsigned long *user_pfn,
>> +                                   long npage, int prot,
>> +                                   unsigned long *phys_pfn)
>> +{
>> +    struct vfio_iommu *iommu = iommu_data;
>> +    struct vfio_domain *domain;
>> +    int i, j, ret;
>> +    long retpage;
>> +    unsigned long remote_vaddr;
>> +    unsigned long *pfn = phys_pfn;
>> +    struct vfio_dma *dma;
>> +    bool do_accounting = false;
>> +
>> +    if (!iommu || !user_pfn || !phys_pfn)
>> +            return -EINVAL;
>> +
>> +    mutex_lock(&iommu->lock);
>> +
>> +    if (!iommu->local_domain) {
>> +            ret = -EINVAL;
>> +            goto pin_done;
>> +    }
>> +
>> +    domain = iommu->local_domain;
>> +
>> +    /*
>> +     * If iommu capable domain exist in the container then all pages are
>> +     * already pinned and accounted. Accouting should be done if there is no
>> +     * iommu capable domain in the container.
>> +     */
>> +    do_accounting = !IS_IOMMU_CAPABLE_DOMAIN_IN_CONTAINER(iommu);
>> +
>> +    for (i = 0; i < npage; i++) {
>> +            struct vfio_pfn *p;
>> +            dma_addr_t iova;
>> +
>> +            iova = user_pfn[i] << PAGE_SHIFT;
>> +
>> +            dma = vfio_find_dma(iommu, iova, 0);
>> +            if (!dma) {
>> +                    ret = -EINVAL;
>> +                    goto pin_unwind;
>> +            }
>> +
>> +            remote_vaddr = dma->vaddr + iova - dma->iova;
>> +
>> +            retpage = __vfio_pin_pages_local(domain, remote_vaddr, prot,
>> +                                             &pfn[i], do_accounting);
> 
> Hi Kirti,
> 
> Here you call __vfio_pin_pages_local() > vaddr_get_pfn() > GUP regardless
> whether the vaddr already pinned or not. That probably means, if the caller 
> calls vfio_pin_pages() with a GPA for multiple times, you get memory leaks.
> 
> GUP always increases the page refcnt.
> 
> FWIW, I would like to have the pfn_list_lock implemented with key == iova,
> so you can always try to find the PFN for a given iova, and pin it only if
> not found.
> 

I didn't get how there would be a memory leak.

Right, GUP increases refcnt, so if vfio_pin_pages() is called for
multiple types for same GPA, refcnt would be incremented. In
vfio_iommu_type1_pin_pages() pinned pages list is maintained with
ref_count. If pfn is already in list, ref_count is incremented and same
is used while unpining pages.

Kirti






reply via email to

[Prev in Thread] Current Thread [Next in Thread]