lkml.org 
[lkml]   [2018]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/2] iommu: Fix iommu_unmap and iommu_unmap_fast return type
From
Date
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 <joro@8bytes.org>
>>> Cc: Alex Williamson <alex.williamson@redhat.com>
>>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@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.

\
 
 \ /
  Last update: 2018-02-01 13:17    [W:1.166 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site