[8/8] iommu/io-pgtable-arm: Prepare for TTBR1 usage

Message ID	20200110152852.24259-9-will@kernel.org (mailing list archive)
State	New, archived
Headers	show Return-Path: <SRS0=nY6Q=27=lists.infradead.org=linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@kernel.org> DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BC9DD2077C From: Will Deacon <will@kernel.org> To: iommu@lists.linux-foundation.org Subject: [PATCH 8/8] iommu/io-pgtable-arm: Prepare for TTBR1 usage Date: Fri, 10 Jan 2020 15:28:52 +0000 Message-Id: <20200110152852.24259-9-will@kernel.org> In-Reply-To: <20200110152852.24259-1-will@kernel.org> References: <20200110152852.24259-1-will@kernel.org> MIME-Version: 1.0 summary: Content analysis details: (-5.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at https://www.dnswl.org/, high trust [198.145.29.99 listed in list.dnswl.org] 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.0 SPF_PASS SPF: sender matches SPF record -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.0 DKIMWL_WL_HIGH DKIMwl.org - Whitelisted High sender Precedence: list Cc: Will Deacon <will@kernel.org>, Jordan Crouse <jcrouse@codeaurora.org>, kernel-team@android.com, Robin Murphy <robin.murphy@arm.com>, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" <linux-arm-kernel-bounces@lists.infradead.org> Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org
Series	Finish off the split page table prep work \| expand [0/8] Finish off the split page table prep work [1/8] iommu/io-pgtable-arm: Rationalise TTBRn handling [2/8] iommu/io-pgtable-arm: Support non-coherent stage-2 page tables [3/8] iommu/io-pgtable-arm: Ensure non-cacheable mappings are Outer Shareable [4/8] iommu/io-pgtable-arm: Ensure ARM_64_LPAE_S2_TCR_RES1 is unsigned [5/8] iommu/io-pgtable-arm: Rationalise TCR handling [6/8] iommu/arm-smmu: Rename public #defines under ARM_SMMU_ namespace [7/8] iommu/io-pgtable-arm: Rationalise VTCR handling [8/8] iommu/io-pgtable-arm: Prepare for TTBR1 usage

Message ID

20200110152852.24259-9-will@kernel.org (mailing list archive)

State

New, archived

Headers

DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BC9DD2077C
From: Will Deacon <will@kernel.org>
To: iommu@lists.linux-foundation.org
Subject: [PATCH 8/8] iommu/io-pgtable-arm: Prepare for TTBR1 usage
Date: Fri, 10 Jan 2020 15:28:52 +0000
Message-Id: <20200110152852.24259-9-will@kernel.org>
In-Reply-To: <20200110152852.24259-1-will@kernel.org>
References: <20200110152852.24259-1-will@kernel.org>
MIME-Version: 1.0
Precedence: list
Cc: Will Deacon <will@kernel.org>, Jordan Crouse <jcrouse@codeaurora.org>,
 kernel-team@android.com, Robin Murphy <robin.murphy@arm.com>,
 linux-arm-kernel@lists.infradead.org
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Sender: "linux-arm-kernel" <linux-arm-kernel-bounces@lists.infradead.org>
Errors-To: 
 linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org

Series

Finish off the split page table prep work | expand

Commit Message

Will Deacon Jan. 10, 2020, 3:28 p.m. UTC

From: Robin Murphy <robin.murphy@arm.com>

Now that we can correctly extract top-level indices without relying on
the remaining upper bits being zero, the only remaining impediments to
using a given table for TTBR1 are the address validation on map/unmap
and the awkward TCR translation granule format. Add a quirk so that we
can do the right thing at those points.

Tested-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
 drivers/iommu/io-pgtable-arm.c | 25 +++++++++++++++++++------
 include/linux/io-pgtable.h     |  4 ++++
 2 files changed, 23 insertions(+), 6 deletions(-)

Comments

Stephan Gerhold Feb. 19, 2020, 12:27 p.m. UTC | #1

Hi Will, Hi Robin,

On Fri, Jan 10, 2020 at 03:28:52PM +0000, Will Deacon wrote:
> From: Robin Murphy <robin.murphy@arm.com>
> 
> Now that we can correctly extract top-level indices without relying on
> the remaining upper bits being zero, the only remaining impediments to
> using a given table for TTBR1 are the address validation on map/unmap
> and the awkward TCR translation granule format. Add a quirk so that we
> can do the right thing at those points.
> 
> Tested-by: Jordan Crouse <jcrouse@codeaurora.org>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  drivers/iommu/io-pgtable-arm.c | 25 +++++++++++++++++++------
>  include/linux/io-pgtable.h     |  4 ++++
>  2 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 846963c87e0f..5e966ef0bacf 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -104,6 +104,10 @@
>  #define ARM_LPAE_TCR_TG0_64K		1
>  #define ARM_LPAE_TCR_TG0_16K		2
>  
> +#define ARM_LPAE_TCR_TG1_16K		1
> +#define ARM_LPAE_TCR_TG1_4K		2
> +#define ARM_LPAE_TCR_TG1_64K		3
> +
>  #define ARM_LPAE_TCR_SH_NS		0
>  #define ARM_LPAE_TCR_SH_OS		2
>  #define ARM_LPAE_TCR_SH_IS		3
> @@ -463,6 +467,7 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
>  	arm_lpae_iopte *ptep = data->pgd;
>  	int ret, lvl = data->start_level;
>  	arm_lpae_iopte prot;
> +	long iaext = (long)iova >> cfg->ias;
>  
>  	/* If no access, then nothing to do */
>  	if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE)))
> @@ -471,7 +476,9 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
>  	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
>  		return -EINVAL;
>  
> -	if (WARN_ON(iova >> data->iop.cfg.ias || paddr >> data->iop.cfg.oas))
> +	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
> +		iaext = ~iaext;
> +	if (WARN_ON(iaext || paddr >> cfg->oas))
>  		return -ERANGE;
>  
>  	prot = arm_lpae_prot_to_pte(data, iommu_prot);
> @@ -637,11 +644,14 @@ static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova,
>  	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
>  	struct io_pgtable_cfg *cfg = &data->iop.cfg;
>  	arm_lpae_iopte *ptep = data->pgd;
> +	long iaext = (long)iova >> cfg->ias;
>  
>  	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
>  		return 0;
>  
> -	if (WARN_ON(iova >> data->iop.cfg.ias))
> +	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
> +		iaext = ~iaext;
> +	if (WARN_ON(iaext))
>  		return 0;
>  
>  	return __arm_lpae_unmap(data, gather, iova, size, data->start_level, ptep);

This part of the patch seems to break io-pgtable-arm.c on ARM32 to some
extent. I have a device using qcom_iommu that could normally run ARM64,
but I'm unable to because of outdated (signed) firmware. :(
So it's running a lot of code that would normally run only on ARM64.

It used to work quite well but now qcom-venus fails to probe on:
    if (WARN_ON(iaext || paddr >> cfg->oas))
(I added some debug prints for clarity.)

    qcom-venus 1d00000.video-codec: Adding to iommu group 0
    arm-lpae io-pgtable: quirks: 0x0
    arm-lpae io-pgtable: arm_lpae_map: iova: 0xdd800000, paddr: 0xebe3f000, iaext: 0xffffffff, paddr >> oas: 0x0
    ------------[ cut here ]------------
    WARNING: CPU: 0 PID: 954 at drivers/iommu/io-pgtable-arm.c:487 arm_lpae_map+0xe4/0x510
    Hardware name: Generic DT based system
    ...
    [<c04bafb0>] (arm_lpae_map) from [<c04bcd6c>] (qcom_iommu_map+0x50/0x78)
    [<c04bcd6c>] (qcom_iommu_map) from [<c04b7290>] (__iommu_map+0xe8/0x1cc)
    [<c04b7290>] (__iommu_map) from [<c04b7bbc>] (__iommu_map_sg+0xa8/0x118)
    [<c04b7bbc>] (__iommu_map_sg) from [<c04b7c64>] (iommu_map_sg_atomic+0x18/0x20)
    [<c04b7c64>] (iommu_map_sg_atomic) from [<c04b94f8>] (iommu_dma_alloc+0x4dc/0x5a0)
    [<c04b94f8>] (iommu_dma_alloc) from [<c0196a50>] (dma_alloc_attrs+0x104/0x114)
    [<c0196a50>] (dma_alloc_attrs) from [<bf302c60>] (venus_hfi_create+0xa4/0x260 [venus_core])
    [<bf302c60>] (venus_hfi_create [venus_core]) from [<bf2fe6cc>] (venus_probe+0x1e4/0x328 [venus_core])
    [<bf2fe6cc>] (venus_probe [venus_core]) from [<c04c8eb4>] (platform_drv_probe+0x48/0x98)
    ...
    Exception stack(0xc2587fa8 to 0xc2587ff0)
    7fa0:                   b6f3dc70 b5051010 b5051010 0017388c b6e645b0 b6e645b0
    7fc0: b6f3dc70 b5051010 00020000 00000080 b6e30790 be84ea54 0046ac91 00000000
    7fe0: b6e75f1c be84e940 b6e5fb51 b6eec8a4
    ---[ end trace 2a0ed284f6d82f16 ]---
    qcom-venus: probe of 1d00000.video-codec failed with error -12

Note that iaext = 0xffffffff != 0.
It seems to be some int/long size problem that only happens with larger
iova addresses on 32-bit systems.

Without the (long) cast for iova everything is working correctly again:

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 983b08477e64..f7ecc763c706 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -468,7 +468,7 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
 	arm_lpae_iopte *ptep = data->pgd;
 	int ret, lvl = data->start_level;
 	arm_lpae_iopte prot;
-	long iaext = (long)iova >> cfg->ias;
+	long iaext = iova >> cfg->ias;
 
 	/* If no access, then nothing to do */
 	if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE)))
@@ -645,7 +645,7 @@ static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova,
 	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
 	struct io_pgtable_cfg *cfg = &data->iop.cfg;
 	arm_lpae_iopte *ptep = data->pgd;
-	long iaext = (long)iova >> cfg->ias;
+	long iaext = iova >> cfg->ias;
 
 	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
 		return 0;

But I'm not sure if this is really the correct "fix"...

This problem can be also easily reproduced by enabling
IOMMU_IO_PGTABLE_LPAE_SELFTEST on ARM32.
(Shouldn't there be some system that runs these automatically? ;))

Without this patch all of them are passing:

    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 32
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 36
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 40
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 42
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 44
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 48
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 32
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 36
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 40
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 42
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 44
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 48
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 32
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 36
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 40
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 42
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 44
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 48
    arm-lpae io-pgtable: selftest: completed with 18 PASS 0 FAIL

But with this patch 6 of them are failing (with a similar warning as
I posted above):

    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 32
    ------------[ cut here ]------------
    WARNING: CPU: 3 PID: 1 at drivers/iommu/io-pgtable-arm.c:482 arm_lpae_map+0xa4/0x4e4
    Hardware name: Generic DT based system
    [<c01104ac>] (unwind_backtrace) from [<c010c0b4>] (show_stack+0x10/0x14)
    [<c010c0b4>] (show_stack) from [<c06cfb34>] (dump_stack+0x90/0xa4)
    [<c06cfb34>] (dump_stack) from [<c012d76c>] (__warn+0xb8/0xd4)
    [<c012d76c>] (__warn) from [<c012d7ec>] (warn_slowpath_fmt+0x64/0xbc)
    [<c012d7ec>] (warn_slowpath_fmt) from [<c04baf70>] (arm_lpae_map+0xa4/0x4e4)
    [<c04baf70>] (arm_lpae_map) from [<c0a1de3c>] (arm_lpae_do_selftests+0x234/0x688)
    [<c0a1de3c>] (arm_lpae_do_selftests) from [<c0102788>] (do_one_initcall+0x50/0x1b8)
    [<c0102788>] (do_one_initcall) from [<c0a0148c>] (kernel_init_freeable+0x1ec/0x25c)
    [<c0a0148c>] (kernel_init_freeable) from [<c06e6dc4>] (kernel_init+0x8/0x110)
    [<c06e6dc4>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
    Exception stack(0xee867fb0 to 0xee867ff8)
    7fa0:                                     00000000 00000000 00000000 00000000
    7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
    7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
    ---[ end trace 89e831c50a111e7c ]---
    ------------[ cut here ]------------
    WARNING: CPU: 3 PID: 1 at drivers/iommu/io-pgtable-arm.c:1182 arm_lpae_do_selftests+0x4b8/0x688
    selftest: test failed for fmt idx 0
    Hardware name: Generic DT based system
    [<c01104ac>] (unwind_backtrace) from [<c010c0b4>] (show_stack+0x10/0x14)
    [<c010c0b4>] (show_stack) from [<c06cfb34>] (dump_stack+0x90/0xa4)
    [<c06cfb34>] (dump_stack) from [<c012d76c>] (__warn+0xb8/0xd4)
    [<c012d76c>] (__warn) from [<c012d820>] (warn_slowpath_fmt+0x98/0xbc)
    [<c012d820>] (warn_slowpath_fmt) from [<c0a1e0c0>] (arm_lpae_do_selftests+0x4b8/0x688)
    [<c0a1e0c0>] (arm_lpae_do_selftests) from [<c0102788>] (do_one_initcall+0x50/0x1b8)
    [<c0102788>] (do_one_initcall) from [<c0a0148c>] (kernel_init_freeable+0x1ec/0x25c)
    [<c0a0148c>] (kernel_init_freeable) from [<c06e6dc4>] (kernel_init+0x8/0x110)
    [<c06e6dc4>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
    Exception stack(0xee867fb0 to 0xee867ff8)
    7fa0:                                     00000000 00000000 00000000 00000000
    7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
    7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
    ---[ end trace 89e831c50a111e7d ]---
    arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 32-bit
    arm-lpae io-pgtable: data: 3 levels, 0x20 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 36
    [warning as above]
    arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 36-bit
    arm-lpae io-pgtable: data: 3 levels, 0x200 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 40
    [warning as above]
    arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 40-bit
    arm-lpae io-pgtable: data: 4 levels, 0x10 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 42
    [warning as above]
    arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 42-bit
    arm-lpae io-pgtable: data: 4 levels, 0x40 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 44
    [warning as above]
    arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 44-bit
    arm-lpae io-pgtable: data: 4 levels, 0x100 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 48
    [warning as above]
    arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 48-bit
    arm-lpae io-pgtable: data: 4 levels, 0x1000 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 32
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 36
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 40
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 42
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 44
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 48
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 32
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 36
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 40
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 42
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 44
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 48
    arm-lpae io-pgtable: selftest: completed with 12 PASS 6 FAIL

Any suggestions how to fix this properly?

Thanks,
Stephan

Robin Murphy Feb. 19, 2020, 5:51 p.m. UTC | #2

On 19/02/2020 12:27 pm, Stephan Gerhold wrote:
> Hi Will, Hi Robin,
> 
> On Fri, Jan 10, 2020 at 03:28:52PM +0000, Will Deacon wrote:
>> From: Robin Murphy <robin.murphy@arm.com>
>>
>> Now that we can correctly extract top-level indices without relying on
>> the remaining upper bits being zero, the only remaining impediments to
>> using a given table for TTBR1 are the address validation on map/unmap
>> and the awkward TCR translation granule format. Add a quirk so that we
>> can do the right thing at those points.
>>
>> Tested-by: Jordan Crouse <jcrouse@codeaurora.org>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> Signed-off-by: Will Deacon <will@kernel.org>
>> ---
>>   drivers/iommu/io-pgtable-arm.c | 25 +++++++++++++++++++------
>>   include/linux/io-pgtable.h     |  4 ++++
>>   2 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
>> index 846963c87e0f..5e966ef0bacf 100644
>> --- a/drivers/iommu/io-pgtable-arm.c
>> +++ b/drivers/iommu/io-pgtable-arm.c
>> @@ -104,6 +104,10 @@
>>   #define ARM_LPAE_TCR_TG0_64K		1
>>   #define ARM_LPAE_TCR_TG0_16K		2
>>   
>> +#define ARM_LPAE_TCR_TG1_16K		1
>> +#define ARM_LPAE_TCR_TG1_4K		2
>> +#define ARM_LPAE_TCR_TG1_64K		3
>> +
>>   #define ARM_LPAE_TCR_SH_NS		0
>>   #define ARM_LPAE_TCR_SH_OS		2
>>   #define ARM_LPAE_TCR_SH_IS		3
>> @@ -463,6 +467,7 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
>>   	arm_lpae_iopte *ptep = data->pgd;
>>   	int ret, lvl = data->start_level;
>>   	arm_lpae_iopte prot;
>> +	long iaext = (long)iova >> cfg->ias;
>>   
>>   	/* If no access, then nothing to do */
>>   	if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE)))
>> @@ -471,7 +476,9 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
>>   	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
>>   		return -EINVAL;
>>   
>> -	if (WARN_ON(iova >> data->iop.cfg.ias || paddr >> data->iop.cfg.oas))
>> +	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
>> +		iaext = ~iaext;
>> +	if (WARN_ON(iaext || paddr >> cfg->oas))
>>   		return -ERANGE;
>>   
>>   	prot = arm_lpae_prot_to_pte(data, iommu_prot);
>> @@ -637,11 +644,14 @@ static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova,
>>   	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
>>   	struct io_pgtable_cfg *cfg = &data->iop.cfg;
>>   	arm_lpae_iopte *ptep = data->pgd;
>> +	long iaext = (long)iova >> cfg->ias;
>>   
>>   	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
>>   		return 0;
>>   
>> -	if (WARN_ON(iova >> data->iop.cfg.ias))
>> +	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
>> +		iaext = ~iaext;
>> +	if (WARN_ON(iaext))
>>   		return 0;
>>   
>>   	return __arm_lpae_unmap(data, gather, iova, size, data->start_level, ptep);
> 
> This part of the patch seems to break io-pgtable-arm.c on ARM32 to some
> extent. I have a device using qcom_iommu that could normally run ARM64,
> but I'm unable to because of outdated (signed) firmware. :(
> So it's running a lot of code that would normally run only on ARM64.
> 
> It used to work quite well but now qcom-venus fails to probe on:
>      if (WARN_ON(iaext || paddr >> cfg->oas))
> (I added some debug prints for clarity.)
> 
>      qcom-venus 1d00000.video-codec: Adding to iommu group 0
>      arm-lpae io-pgtable: quirks: 0x0
>      arm-lpae io-pgtable: arm_lpae_map: iova: 0xdd800000, paddr: 0xebe3f000, iaext: 0xffffffff, paddr >> oas: 0x0
>      ------------[ cut here ]------------
>      WARNING: CPU: 0 PID: 954 at drivers/iommu/io-pgtable-arm.c:487 arm_lpae_map+0xe4/0x510
>      Hardware name: Generic DT based system
>      ...
>      [<c04bafb0>] (arm_lpae_map) from [<c04bcd6c>] (qcom_iommu_map+0x50/0x78)
>      [<c04bcd6c>] (qcom_iommu_map) from [<c04b7290>] (__iommu_map+0xe8/0x1cc)
>      [<c04b7290>] (__iommu_map) from [<c04b7bbc>] (__iommu_map_sg+0xa8/0x118)
>      [<c04b7bbc>] (__iommu_map_sg) from [<c04b7c64>] (iommu_map_sg_atomic+0x18/0x20)
>      [<c04b7c64>] (iommu_map_sg_atomic) from [<c04b94f8>] (iommu_dma_alloc+0x4dc/0x5a0)
>      [<c04b94f8>] (iommu_dma_alloc) from [<c0196a50>] (dma_alloc_attrs+0x104/0x114)
>      [<c0196a50>] (dma_alloc_attrs) from [<bf302c60>] (venus_hfi_create+0xa4/0x260 [venus_core])
>      [<bf302c60>] (venus_hfi_create [venus_core]) from [<bf2fe6cc>] (venus_probe+0x1e4/0x328 [venus_core])
>      [<bf2fe6cc>] (venus_probe [venus_core]) from [<c04c8eb4>] (platform_drv_probe+0x48/0x98)
>      ...
>      Exception stack(0xc2587fa8 to 0xc2587ff0)
>      7fa0:                   b6f3dc70 b5051010 b5051010 0017388c b6e645b0 b6e645b0
>      7fc0: b6f3dc70 b5051010 00020000 00000080 b6e30790 be84ea54 0046ac91 00000000
>      7fe0: b6e75f1c be84e940 b6e5fb51 b6eec8a4
>      ---[ end trace 2a0ed284f6d82f16 ]---
>      qcom-venus: probe of 1d00000.video-codec failed with error -12
> 
> Note that iaext = 0xffffffff != 0.
> It seems to be some int/long size problem that only happens with larger
> iova addresses on 32-bit systems.
> 
> Without the (long) cast for iova everything is working correctly again:
> 
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 983b08477e64..f7ecc763c706 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -468,7 +468,7 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
>   	arm_lpae_iopte *ptep = data->pgd;
>   	int ret, lvl = data->start_level;
>   	arm_lpae_iopte prot;
> -	long iaext = (long)iova >> cfg->ias;
> +	long iaext = iova >> cfg->ias;
>   
>   	/* If no access, then nothing to do */
>   	if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE)))
> @@ -645,7 +645,7 @@ static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova,
>   	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
>   	struct io_pgtable_cfg *cfg = &data->iop.cfg;
>   	arm_lpae_iopte *ptep = data->pgd;
> -	long iaext = (long)iova >> cfg->ias;
> +	long iaext = iova >> cfg->ias;
>   
>   	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
>   		return 0;
> 
> But I'm not sure if this is really the correct "fix"...

No, that would end up breaking the TTBR1 case. The quirk isn't intended 
to be supported on 32-bit platforms, but somehow it's slipped through 
that this sign-extension still affects them (and goes wrong to boot), 
sorry about that.

I think quick fixes would be either to replace the "(long)" cast with 
"(s64)(u64)", or to just elide the iaext check altogether when 32-bit 
longs make it impossible to pass an out-of-range IOVA anyway. I'll have 
a think and see what works out nicest.

> This problem can be also easily reproduced by enabling
> IOMMU_IO_PGTABLE_LPAE_SELFTEST on ARM32.
> (Shouldn't there be some system that runs these automatically? ;))
> 
> Without this patch all of them are passing:
> 
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 32
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 36
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 40
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 42
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 44
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 48
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 32
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 36
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 40
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 42
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 44
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 48
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 32
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 36
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 40
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 42
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 44
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 48
>      arm-lpae io-pgtable: selftest: completed with 18 PASS 0 FAIL

Hmm, we probably shouldn't be running IAS > 32 tests on 32-bit, given 
that such configs are effectively impossible to use.

Robin.

> 
> But with this patch 6 of them are failing (with a similar warning as
> I posted above):
> 
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 32
>      ------------[ cut here ]------------
>      WARNING: CPU: 3 PID: 1 at drivers/iommu/io-pgtable-arm.c:482 arm_lpae_map+0xa4/0x4e4
>      Hardware name: Generic DT based system
>      [<c01104ac>] (unwind_backtrace) from [<c010c0b4>] (show_stack+0x10/0x14)
>      [<c010c0b4>] (show_stack) from [<c06cfb34>] (dump_stack+0x90/0xa4)
>      [<c06cfb34>] (dump_stack) from [<c012d76c>] (__warn+0xb8/0xd4)
>      [<c012d76c>] (__warn) from [<c012d7ec>] (warn_slowpath_fmt+0x64/0xbc)
>      [<c012d7ec>] (warn_slowpath_fmt) from [<c04baf70>] (arm_lpae_map+0xa4/0x4e4)
>      [<c04baf70>] (arm_lpae_map) from [<c0a1de3c>] (arm_lpae_do_selftests+0x234/0x688)
>      [<c0a1de3c>] (arm_lpae_do_selftests) from [<c0102788>] (do_one_initcall+0x50/0x1b8)
>      [<c0102788>] (do_one_initcall) from [<c0a0148c>] (kernel_init_freeable+0x1ec/0x25c)
>      [<c0a0148c>] (kernel_init_freeable) from [<c06e6dc4>] (kernel_init+0x8/0x110)
>      [<c06e6dc4>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
>      Exception stack(0xee867fb0 to 0xee867ff8)
>      7fa0:                                     00000000 00000000 00000000 00000000
>      7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>      7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>      ---[ end trace 89e831c50a111e7c ]---
>      ------------[ cut here ]------------
>      WARNING: CPU: 3 PID: 1 at drivers/iommu/io-pgtable-arm.c:1182 arm_lpae_do_selftests+0x4b8/0x688
>      selftest: test failed for fmt idx 0
>      Hardware name: Generic DT based system
>      [<c01104ac>] (unwind_backtrace) from [<c010c0b4>] (show_stack+0x10/0x14)
>      [<c010c0b4>] (show_stack) from [<c06cfb34>] (dump_stack+0x90/0xa4)
>      [<c06cfb34>] (dump_stack) from [<c012d76c>] (__warn+0xb8/0xd4)
>      [<c012d76c>] (__warn) from [<c012d820>] (warn_slowpath_fmt+0x98/0xbc)
>      [<c012d820>] (warn_slowpath_fmt) from [<c0a1e0c0>] (arm_lpae_do_selftests+0x4b8/0x688)
>      [<c0a1e0c0>] (arm_lpae_do_selftests) from [<c0102788>] (do_one_initcall+0x50/0x1b8)
>      [<c0102788>] (do_one_initcall) from [<c0a0148c>] (kernel_init_freeable+0x1ec/0x25c)
>      [<c0a0148c>] (kernel_init_freeable) from [<c06e6dc4>] (kernel_init+0x8/0x110)
>      [<c06e6dc4>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
>      Exception stack(0xee867fb0 to 0xee867ff8)
>      7fa0:                                     00000000 00000000 00000000 00000000
>      7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>      7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>      ---[ end trace 89e831c50a111e7d ]---
>      arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 32-bit
>      arm-lpae io-pgtable: data: 3 levels, 0x20 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 36
>      [warning as above]
>      arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 36-bit
>      arm-lpae io-pgtable: data: 3 levels, 0x200 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 40
>      [warning as above]
>      arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 40-bit
>      arm-lpae io-pgtable: data: 4 levels, 0x10 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 42
>      [warning as above]
>      arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 42-bit
>      arm-lpae io-pgtable: data: 4 levels, 0x40 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 44
>      [warning as above]
>      arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 44-bit
>      arm-lpae io-pgtable: data: 4 levels, 0x100 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 48
>      [warning as above]
>      arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 48-bit
>      arm-lpae io-pgtable: data: 4 levels, 0x1000 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 32
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 36
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 40
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 42
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 44
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 48
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 32
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 36
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 40
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 42
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 44
>      arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 48
>      arm-lpae io-pgtable: selftest: completed with 12 PASS 6 FAIL
> 
> Any suggestions how to fix this properly?
> 
> Thanks,
> Stephan
>

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 846963c87e0f..5e966ef0bacf 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -104,6 +104,10 @@ 
 #define ARM_LPAE_TCR_TG0_64K		1
 #define ARM_LPAE_TCR_TG0_16K		2
 
+#define ARM_LPAE_TCR_TG1_16K		1
+#define ARM_LPAE_TCR_TG1_4K		2
+#define ARM_LPAE_TCR_TG1_64K		3
+
 #define ARM_LPAE_TCR_SH_NS		0
 #define ARM_LPAE_TCR_SH_OS		2
 #define ARM_LPAE_TCR_SH_IS		3
@@ -463,6 +467,7 @@  static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
 	arm_lpae_iopte *ptep = data->pgd;
 	int ret, lvl = data->start_level;
 	arm_lpae_iopte prot;
+	long iaext = (long)iova >> cfg->ias;
 
 	/* If no access, then nothing to do */
 	if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE)))
@@ -471,7 +476,9 @@  static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
 	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
 		return -EINVAL;
 
-	if (WARN_ON(iova >> data->iop.cfg.ias || paddr >> data->iop.cfg.oas))
+	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
+		iaext = ~iaext;
+	if (WARN_ON(iaext || paddr >> cfg->oas))
 		return -ERANGE;
 
 	prot = arm_lpae_prot_to_pte(data, iommu_prot);
@@ -637,11 +644,14 @@  static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova,
 	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
 	struct io_pgtable_cfg *cfg = &data->iop.cfg;
 	arm_lpae_iopte *ptep = data->pgd;
+	long iaext = (long)iova >> cfg->ias;
 
 	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
 		return 0;
 
-	if (WARN_ON(iova >> data->iop.cfg.ias))
+	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
+		iaext = ~iaext;
+	if (WARN_ON(iaext))
 		return 0;
 
 	return __arm_lpae_unmap(data, gather, iova, size, data->start_level, ptep);
@@ -777,9 +787,11 @@  arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg, void *cookie)
 	u64 reg;
 	struct arm_lpae_io_pgtable *data;
 	typeof(&cfg->arm_lpae_s1_cfg.tcr) tcr = &cfg->arm_lpae_s1_cfg.tcr;
+	bool tg1;
 
 	if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |
-			    IO_PGTABLE_QUIRK_NON_STRICT))
+			    IO_PGTABLE_QUIRK_NON_STRICT |
+			    IO_PGTABLE_QUIRK_ARM_TTBR1))
 		return NULL;
 
 	data = arm_lpae_alloc_pgtable(cfg);
@@ -797,15 +809,16 @@  arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg, void *cookie)
 		tcr->orgn = ARM_LPAE_TCR_RGN_NC;
 	}
 
+	tg1 = cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1;
 	switch (ARM_LPAE_GRANULE(data)) {
 	case SZ_4K:
-		tcr->tg = ARM_LPAE_TCR_TG0_4K;
+		tcr->tg = tg1 ? ARM_LPAE_TCR_TG1_4K : ARM_LPAE_TCR_TG0_4K;
 		break;
 	case SZ_16K:
-		tcr->tg = ARM_LPAE_TCR_TG0_16K;
+		tcr->tg = tg1 ? ARM_LPAE_TCR_TG1_16K : ARM_LPAE_TCR_TG0_16K;
 		break;
 	case SZ_64K:
-		tcr->tg = ARM_LPAE_TCR_TG0_64K;
+		tcr->tg = tg1 ? ARM_LPAE_TCR_TG1_64K : ARM_LPAE_TCR_TG0_64K;
 		break;
 	}
 
diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
index 40c1b7745fb6..53d53c6c2be9 100644
--- a/include/linux/io-pgtable.h
+++ b/include/linux/io-pgtable.h
@@ -83,12 +83,16 @@  struct io_pgtable_cfg {
 	 * IO_PGTABLE_QUIRK_NON_STRICT: Skip issuing synchronous leaf TLBIs
 	 *	on unmap, for DMA domains using the flush queue mechanism for
 	 *	delayed invalidation.
+	 *
+	 * IO_PGTABLE_QUIRK_ARM_TTBR1: (ARM LPAE format) Configure the table
+	 *	for use in the upper half of a split address space.
 	 */
 	#define IO_PGTABLE_QUIRK_ARM_NS		BIT(0)
 	#define IO_PGTABLE_QUIRK_NO_PERMS	BIT(1)
 	#define IO_PGTABLE_QUIRK_TLBI_ON_MAP	BIT(2)
 	#define IO_PGTABLE_QUIRK_ARM_MTK_EXT	BIT(3)
 	#define IO_PGTABLE_QUIRK_NON_STRICT	BIT(4)
+	#define IO_PGTABLE_QUIRK_ARM_TTBR1	BIT(5)
 	unsigned long			quirks;
 	unsigned long			pgsize_bitmap;
 	unsigned int			ias;

[8/8] iommu/io-pgtable-arm: Prepare for TTBR1 usage

Commit Message

Comments

Patch