Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RFC v1 5/8] mshv: add paravirtualized IOMMU support
To:     Wei Liu <wei.liu@kernel.org>
Cc:     Linux on Hyper-V List <linux-hyperv@vger.kernel.org>,
        virtualization@lists.linux-foundation.org,
        Linux Kernel List <linux-kernel@vger.kernel.org>,
        Michael Kelley <mikelley@microsoft.com>,
        Vineeth Pillai <viremana@linux.microsoft.com>,
        Sunil Muthuswamy <sunilmut@microsoft.com>,
        Nuno Das Neves <nunodasneves@linux.microsoft.com>,
        pasha.tatashin@soleen.com, Joerg Roedel <joro@8bytes.org>,
        Will Deacon <will@kernel.org>,
        "K. Y. Srinivasan" <kys@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>,
        Stephen Hemminger <sthemmin@microsoft.com>,
        Dexuan Cui <decui@microsoft.com>,
        "open list:IOMMU DRIVERS" <iommu@lists.linux-foundation.org>
References: <20210709114339.3467637-1-wei.liu@kernel.org>
 <20210709114339.3467637-6-wei.liu@kernel.org>
 <77670985-2a1b-7bbd-2ede-4b7810c3e220@linux.microsoft.com>
 <20210803214718.hnp3ejs35lh455fw@liuwe-devbox-debian-v2>
From:   Praveen Kumar <kumarpraveen@linux.microsoft.com>
Message-ID: <562f4c31-1ea5-ea56-ac0a-6bd80b61c3df@linux.microsoft.com>
Date:   Wed, 4 Aug 2021 12:13:42 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.12.0
MIME-Version: 1.0
In-Reply-To: <20210803214718.hnp3ejs35lh455fw@liuwe-devbox-debian-v2>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 04-08-2021 03:17, Wei Liu wrote:
>>> +static size_t hv_iommu_unmap(struct iommu_domain *d, unsigned long iova,
>>> +			   size_t size, struct iommu_iotlb_gather *gather)
>>> +{
>>> +	size_t unmapped;
>>> +	struct hv_iommu_domain *domain = to_hv_iommu_domain(d);
>>> +	unsigned long flags, npages;
>>> +	struct hv_input_unmap_device_gpa_pages *input;
>>> +	u64 status;
>>> +
>>> +	unmapped = hv_iommu_del_mappings(domain, iova, size);
>>> +	if (unmapped < size)
>>> +		return 0;
>> Is there a case where unmapped > 0 && unmapped < size ?
>>
> There could be such a case -- hv_iommu_del_mappings' return value is >= 0.
> Is there a problem with this predicate?

What I understand, if we are unmapping and return 0, means nothing was unmapped, and will that not cause any corruption or illegal access of unmapped memory later?
From __iommu_unmap
...
    13         while (unmapped < size) {
    12                 size_t pgsize = iommu_pgsize(domain, iova, size - unmapped);
    11
    10                 unmapped_page = ops->unmap(domain, iova, pgsize, iotlb_gather);
     9                 if (!unmapped_page)
     8                         break;		<<< we just break here, thinking there is nothing unmapped, but actually hv_iommu_del_mappings has removed some pages.
     7
     6                 pr_debug("unmapped: iova 0x%lx size 0x%zx\n",
     5                         ¦iova, unmapped_page);
     4
     3                 iova += unmapped_page;
     2                 unmapped += unmapped_page;
     1         }
...

Am I missing something ?

Regards,

~Praveen.