On 01/02/18 05:03, Suravee Suthikulpanit wrote:
Hi Robin,

On 2/1/18 1:02 AM, Robin Murphy wrote:
Hi Suravee,

On 31/01/18 01:48, Suravee Suthikulpanit wrote:
Currently, iommu_unmap and iommu_unmap_fast return unmapped
pages with size_t.  However, the actual value returned could
be error codes (< 0), which can be misinterpreted as large
number of unmapped pages. Therefore, change the return type to ssize_t.

Cc: Joerg Roedel <j...@8bytes.org>
Cc: Alex Williamson <alex.william...@redhat.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
---
  drivers/iommu/amd_iommu.c   |  6 +++---
  drivers/iommu/intel-iommu.c |  4 ++--

Er, there are a few more drivers than that implementing iommu_ops ;)

Ahh right.

It seems like it might be more sensible to fix the single instance of a driver returning -EINVAL (which appears to be a "should never happen if used correctly" kinda thing anyway) and leave the API-internal callback prototype as-is. I do agree the inconsistency of iommu_unmap() itself wants sorting, though (particularly the !IOMMU_API stubs which are wrong either way).

Robin.

Make sense. I'll leave the API alone, and change the code to not returning error then.

Actually, on a second look I think that check in amd_iommu is 99% redundant anyway, since PAGE_MODE_NONE is only normally set for IOMMU_DOMAIN_IDENTITY domains, thus iommu_unmap() would have bailed out from the __IOMMU_DOMAIN_PAGING check before ops->unmap could be called. AFAICS the only way to hit it at all is if a caller somehow managed to get hold of the dev_state->domain set up in amd_iommu_init_device(), then tried to unmap something from that, which seems such a very wrong thing to do it should probably just crash and burn with extreme prejudice anyway.

Cheers,
Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to