linux-kernel - Re: [PATCH] iommu/arm-smmu: Return IOVA in iova_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 24 Apr 2017 21:23:16 +0530
From:   Sunil Kovvuri <sunil.kovvuri@...il.com>
To:     Will Deacon <will.deacon@....com>
Cc:     iommu@...ts.linux-foundation.org,
        LAKML <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>, robert.richter@...ium.com,
        Sunil Goutham <sgoutham@...ium.com>
Subject: Re: [PATCH] iommu/arm-smmu: Return IOVA in iova_to_phys when SMMU is bypassed

On Mon, Apr 24, 2017 at 8:14 PM, Will Deacon <will.deacon@....com> wrote:
> On Mon, Apr 17, 2017 at 05:27:26PM +0530, sunil.kovvuri@...il.com wrote:
>> From: Sunil Goutham <sgoutham@...ium.com>
>>
>> For software initiated address translation, when domain type is
>> IOMMU_DOMAIN_IDENTITY i.e SMMU is bypassed, mimic HW behavior
>> i.e return the same IOVA as translated address.
>>
>> This patch is an extension to Will Deacon's patchset
>> "Implement SMMU passthrough using the default domain".
>
> Are you actually seeing an issue here? If so, why isn't SMMUv3 affected too?
Yes and SMMUv3 should also be effected but as of now I don't see any use case.
If needed, i can re-submit the patch with changes in SMMUv3 as well.

>
>> Signed-off-by: Sunil Goutham <sgoutham@...ium.com>
>> ---
>>  drivers/iommu/arm-smmu.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 41afb07..2f4a130 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -1405,6 +1405,9 @@ static phys_addr_t arm_smmu_iova_to_phys(struct iommu_domain *domain,
>>       struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>>       struct io_pgtable_ops *ops= smmu_domain->pgtbl_ops;
>>
>> +     if (domain->type == IOMMU_DOMAIN_IDENTITY)
>> +             return iova;
>> +
>>       if (!ops)
>>               return 0;
>
> I'd have thought ops would be NULL, since arm_smmu_init_domain_context
> doesn't allocate them for an identity domain.
Yes ops is set to NULL.

>
> I don't understand this patch. Please can you explain the problem more
> clearly?
AFAIK for any driver outside IOMMU there is only one way to identify
if device is attached to
IOMMU or not and that is by checking iommu_domain. And I don't think
it would be appropriate
for the driver to check domain->type before calling 'iommu_iova_to_phys()' API.

The difference between IOMMU disabled and IOMMU being in passthrough
mode is that, in the
later case device is still attached to default domain but in former's
case it's NULL. So there is no
way to differentiate for the external driver whether IOMMU is in
passthrough mode or DMA mode.

And since ops is NULL in passthrough mode, 'iommu_iova_to_phys()' will
return zero.

Use case for your reference
https://lkml.org/lkml/2017/3/7/299
This driver is for a NIC interface on platform which supports SMMUv2.

Let me know if any more details are needed.

Thanks,
Sunil.

>
> Will