On Wed, Aug 04, 2021 at 12:13:42PM +0530, Praveen Kumar wrote:
> 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
Those pages are not really unmapped. The hypercall is skipped.
> ...
> 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.
