linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* next-20250702 WARNING iommu io-pgtable-arm.c at arm_lpae_map_pages qcom_iommu_map
@ 2025-07-08 20:56 Naresh Kamboju
  2025-07-09  0:25 ` Jason Gunthorpe
  0 siblings, 1 reply; 7+ messages in thread
From: Naresh Kamboju @ 2025-07-08 20:56 UTC (permalink / raw)
  To: open list, iommu, lkft-triage, Linux Regressions
  Cc: Jason Gunthorpe, Nicolin Chen, Jean-Philippe Brucker,
	Anders Roxell, Ben Copeland, Arnd Bergmann, Dan Carpenter

Regression identified while booting the Dragonboard 410c (Qualcomm
APQ8016 SBC) using the Linux next-20250702 kernel tag. During device
initialization, the kernel triggers a WARNING in the arm_lpae_map_pages()
function, which is part of the IOMMU subsystem. The call trace also involves
qcom_iommu_map().

Test environments:
- Dragonboard-410c

Regression Analysis:
- New regression? Yes
- Reproducibility? Yes

Boot regression: next-20250702 WARNING iommu io-pgtable-arm.c at
arm_lpae_map_pages qcom_iommu_map

Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>

List of suspected patches with recent changes.
 * https://lore.kernel.org/all/0-v2-68a2e1ba507c+1fb-iommu_rm_ops_pgsize_jgg@nvidia.com/

## Boot log

[    9.504700] ------------[ cut here ]------------
[    9.509432] WARNING: drivers/iommu/io-pgtable-arm.c:569 at
arm_lpae_map_pages+0x30/0x1cc, CPU#0: udev-worker)/216
[    9.514301] Modules linked in: snd_soc_core venus_core(+) qrtr
videobuf2_dma_sg qcom_q6v5_mss v4l2_fwnode snd_compress adv7511
llcc_qcom snd_pcm_dmaengine snd_pcm ocmem qcom_pil_info qcom_q6v5
v4l2_async drm_exec snd_timer qcom_sysmon v4l2_mem2mem gpu_sched
videobuf2_memops snd videobuf2_v4l2 qcom_common drm_dp_aux_bus
qcom_spmi_temp_alarm qcom_pon qcom_spmi_vadc drm_display_helper
rtc_pm8xxx soundcore videodev qcom_glink_smem qcom_vadc_common
mdt_loader cec qmi_helpers qnoc_msm8916 videobuf2_common
drm_client_lib mc phy_qcom_usb_hs qcom_stats qcom_rng ramoops socinfo
rpmsg_ctrl rmtfs_mem rpmsg_char display_connector reed_solomon
drm_kms_helper fuse drm backlight ip_tables x_tables
[    9.562593] CPU: 0 UID: 0 PID: 216 Comm: (udev-worker) Not tainted
6.16.0-rc4-next-20250702 #1 PREEMPT
[    9.584779] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
[    9.594059] pstate: 000000c5 (nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    9.601002] pc : arm_lpae_map_pages
(drivers/iommu/io-pgtable-arm.c:569 (discriminator 7))
[    9.607682] lr : qcom_iommu_map (drivers/iommu/arm/arm-smmu/qcom_iommu.c:441)
[    9.612196] sp : ffff800083a9b4e0
[    9.616274] x29: ffff800083a9b4e0 x28: 0000000000000004 x27: 0000000000200000
[    9.619579] x26: 0000000000000003 x25: ffff000004a2ec78 x24: ffff800083a9b550
[    9.626698] x23: 0000000000000002 x22: 0000000000100000 x21: 000000008f400000
[    9.633816] x20: 0000000000000000 x19: ffff000004a2eb48 x18: 0000000000000000
[    9.640934] x17: ffff000003807000 x16: ffff000003806e00 x15: ffff000004a2ec78
[    9.648051] x14: ffff000003ef8000 x13: 0000000000000001 x12: ffff000004a2ec10
[    9.655170] x11: 0000000000000004 x10: 0000000000000820 x9 : 0000000000000000
[    9.662287] x8 : ffff8000809fe7a4 x7 : ffff800083a9b550 x6 : 0000000000001000
[    9.669406] x5 : 0000000000000003 x4 : 0000000000000002 x3 : 0000000000100000
[    9.676530] x2 : 000000008f400000 x1 : 00000000dd800000 x0 : ffff000004a2ec00
[    9.683643] Call trace:
[    9.690752] arm_lpae_map_pages (drivers/iommu/io-pgtable-arm.c:569
(discriminator 7)) (P)
[    9.693013] iommu_map_nosync (drivers/iommu/iommu.c:2505)
[    9.697524] iommu_map_sg (drivers/iommu/iommu.c:2677)
[    9.701604] __iommu_dma_alloc_noncontiguous (drivers/iommu/dma-iommu.c:982)
[    9.705253] iommu_dma_alloc (drivers/iommu/dma-iommu.c:1006
drivers/iommu/dma-iommu.c:1650)
[    9.710546] dma_alloc_attrs (kernel/dma/mapping.c:638)
[    9.714452] venus_hfi_create+0xa8/0x248 venus_core
[    9.718015] hfi_create+0x54/0x6c venus_core
[    9.723221] venus_probe+0x254/0x574 venus_core
[    9.727563] platform_probe (drivers/base/platform.c:1404)
[    9.732333] really_probe (drivers/base/dd.c:579 drivers/base/dd.c:657)
[    9.735979] __driver_probe_device (drivers/base/dd.c:799)
[    9.739626] driver_probe_device (drivers/base/dd.c:829)
[    9.743879] __driver_attach (drivers/base/dd.c:1216 drivers/base/dd.c:1155)
[    9.747871] bus_for_each_dev (drivers/base/bus.c:370)
[    9.751690] driver_attach (drivers/base/dd.c:1234)
[    9.755510] bus_add_driver (drivers/base/bus.c:678)
[    9.759330] driver_register (drivers/base/driver.c:249)
[    9.762889] __platform_driver_register (drivers/base/platform.c:868)
[    9.766623] qcom_venus_driver_init+0x20/0xfc0 venus_core
[    9.771486] do_one_initcall (init/main.c:1269)
[    9.776865] do_init_module (kernel/module/main.c:3046)
[    9.780685] load_module (kernel/module/main.c:3516)
[    9.784503] init_module_from_file (kernel/module/main.c:3709)
[    9.788237] __arm64_sys_finit_module (kernel/module/main.c:3720
kernel/module/main.c:3746 kernel/module/main.c:3730
kernel/module/main.c:3730)
[    9.792668] invoke_syscall (arch/arm64/include/asm/current.h:19
arch/arm64/kernel/syscall.c:54)
[    9.797264] el0_svc_common.constprop.0 (arch/arm64/kernel/syscall.c:139)
[    9.800911] do_el0_svc (arch/arm64/kernel/syscall.c:152)
[    9.805683] el0_svc (arch/arm64/include/asm/irqflags.h:55
arch/arm64/include/asm/irqflags.h:76
arch/arm64/kernel/entry-common.c:169
arch/arm64/kernel/entry-common.c:182
arch/arm64/kernel/entry-common.c:772)
[    9.808982] el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:791)
[    9.811934] el0t_64_sync (arch/arm64/kernel/entry.S:600)
[    9.816362] ---[ end trace 0000000000000000 ]---

## Source
* Kernel version: 6.16.0-rc4-next-20250702
* Git tree: https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next.git
* Git sha: 50c8770a42faf8b1c7abe93e7c114337f580a97d
* Git describe: next-20250702
* Project: https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20250702
* Architectures: arm64
* Toolchains: gcc-13
* Kconfigs: gcc-13-lkftconfig

## Build
* Test log: https://qa-reports.linaro.org/api/testruns/28989497/log_file/
* Test LAVA log: https://lkft.validation.linaro.org/scheduler/job/8339869#L3862
* Test details:
https://regressions.linaro.org/lkft/linux-next-master/next-20250702/boot/gcc-13-lkftconfig-no-kselftest-frag/
* Test plan: https://tuxapi.tuxsuite.com/v1/groups/linaro/projects/lkft/tests/2zJgUBCVJbqFPufxneFurZszovX
* Build link: https://storage.tuxsuite.com/public/linaro/lkft/builds/2zJgRgltAwUHEijfEA14MY3VTTF/
* Kernel config:
https://storage.tuxsuite.com/public/linaro/lkft/builds/2zJgRgltAwUHEijfEA14MY3VTTF/config

--
Linaro LKFT
https://lkft.linaro.org

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: next-20250702 WARNING iommu io-pgtable-arm.c at arm_lpae_map_pages qcom_iommu_map
  2025-07-08 20:56 next-20250702 WARNING iommu io-pgtable-arm.c at arm_lpae_map_pages qcom_iommu_map Naresh Kamboju
@ 2025-07-09  0:25 ` Jason Gunthorpe
  2025-07-09 10:44   ` Naresh Kamboju
  0 siblings, 1 reply; 7+ messages in thread
From: Jason Gunthorpe @ 2025-07-09  0:25 UTC (permalink / raw)
  To: Naresh Kamboju
  Cc: open list, iommu, lkft-triage, Linux Regressions, Nicolin Chen,
	Jean-Philippe Brucker, Anders Roxell, Ben Copeland, Arnd Bergmann,
	Dan Carpenter

On Wed, Jul 09, 2025 at 02:26:20AM +0530, Naresh Kamboju wrote:
> Regression identified while booting the Dragonboard 410c (Qualcomm
> APQ8016 SBC) using the Linux next-20250702 kernel tag. During device
> initialization, the kernel triggers a WARNING in the arm_lpae_map_pages()
> function, which is part of the IOMMU subsystem. The call trace also involves
> qcom_iommu_map().
> 
> Test environments:
> - Dragonboard-410c
> 
> Regression Analysis:
> - New regression? Yes
> - Reproducibility? Yes
> 
> Boot regression: next-20250702 WARNING iommu io-pgtable-arm.c at
> arm_lpae_map_pages qcom_iommu_map
> 
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> 
> List of suspected patches with recent changes.
>  * https://lore.kernel.org/all/0-v2-68a2e1ba507c+1fb-iommu_rm_ops_pgsize_jgg@nvidia.com/

Can you test this fix please:

--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -229,7 +229,7 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
                goto out_unlock;
 
        pgtbl_cfg = (struct io_pgtable_cfg) {
-               .pgsize_bitmap  = domain->pgsize_bitmap,
+               .pgsize_bitmap  = SZ_4K | SZ_64K | SZ_1M | SZ_16M,
                .ias            = 32,
                .oas            = 40,
                .tlb            = &qcom_flush_ops,
@@ -246,6 +246,8 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
                goto out_clear_iommu;
        }
 
+       /* Update the domain's page sizes to reflect the page table format */
+       domain->pgsize_bitmap = pgtbl_cfg.pgsize_bitmap;
        domain->geometry.aperture_end = (1ULL << pgtbl_cfg.ias) - 1;
        domain->geometry.force_aperture = true;
 
@@ -335,7 +337,6 @@ static struct iommu_domain *qcom_iommu_domain_alloc_paging(struct device *dev)
 
        mutex_init(&qcom_domain->init_mutex);
        spin_lock_init(&qcom_domain->pgtbl_lock);
-       qcom_domain->domain.pgsize_bitmap = SZ_4K | SZ_64K | SZ_1M | SZ_16M;
 
        return &qcom_domain->domain;
 }

Of all the drivers qcom is the only one that uses the 64 bit arm page
table, 4 & 64k sizes, and was using the ops global. The io_pgtable
code will remove one of the two depending on PAGE_SIZE which makes
things inconsistent and hits that warn.

Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: next-20250702 WARNING iommu io-pgtable-arm.c at arm_lpae_map_pages qcom_iommu_map
  2025-07-09  0:25 ` Jason Gunthorpe
@ 2025-07-09 10:44   ` Naresh Kamboju
  2025-07-09 16:26     ` Jason Gunthorpe
  0 siblings, 1 reply; 7+ messages in thread
From: Naresh Kamboju @ 2025-07-09 10:44 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: open list, iommu, lkft-triage, Linux Regressions, Nicolin Chen,
	Jean-Philippe Brucker, Anders Roxell, Ben Copeland, Arnd Bergmann,
	Dan Carpenter

On Wed, 9 Jul 2025 at 05:55, Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Wed, Jul 09, 2025 at 02:26:20AM +0530, Naresh Kamboju wrote:
> > Regression identified while booting the Dragonboard 410c (Qualcomm
> > APQ8016 SBC) using the Linux next-20250702 kernel tag. During device
> > initialization, the kernel triggers a WARNING in the arm_lpae_map_pages()
> > function, which is part of the IOMMU subsystem. The call trace also involves
> > qcom_iommu_map().
> >
> > Test environments:
> > - Dragonboard-410c
> >
> > Regression Analysis:
> > - New regression? Yes
> > - Reproducibility? Yes
> >
> > Boot regression: next-20250702 WARNING iommu io-pgtable-arm.c at
> > arm_lpae_map_pages qcom_iommu_map
> >
> > Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> >
> > List of suspected patches with recent changes.
> >  * https://lore.kernel.org/all/0-v2-68a2e1ba507c+1fb-iommu_rm_ops_pgsize_jgg@nvidia.com/
>
> Can you test this fix please:


I have tested this patch on top of Linux next-20250702 tag,
and found kernel warning,

[    1.510468] ------------[ cut here ]------------
[    1.516302] WARNING: drivers/iommu/iommu.c:1142 at
iommu_create_device_direct_mappings+0x240/0x258, CPU#1: swapper/0/1
[    1.521001] Modules linked in:
[    1.531485] CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted
6.16.0-rc4-next-20250702 #1 PREEMPT
[    1.534538] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
[    1.543473] pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    1.550241] pc : iommu_create_device_direct_mappings
(drivers/iommu/iommu.c:1142 (discriminator 7))
[    1.556924] lr : iommu_setup_default_domain
(drivers/iommu/iommu.c:2992 (discriminator 1))
[    1.563170] sp : ffff80008002b9c0
[    1.568113] x29: ffff80008002b9e0 x28: 0000000000000000 x27: ffff80008174e2c0
[    1.571596] x26: ffff000004c58030 x25: ffff800081d75228 x24: ffff80008221eba4
[    1.578714] x23: ffff000003d99410 x22: ffff80008002b9c8 x21: ffff000003ce5900
[    1.585833] x20: ffff000003ce5948 x19: ffff000002e6d5a0 x18: 0000000000000000
[    1.592951] x17: ffff000003d4a000 x16: ffff000002c37e00 x15: 07690720076f0774
[    1.600068] x14: 0000000000000000 x13: ffff800082237670 x12: 000000000003786e
[    1.607185] x11: 0000000000000115 x10: 0000000000103758 x9 : 0000000000000000
[    1.614303] x8 : ffff000003ce5d00 x7 : 0000000000000000 x6 : 000000000000003f
[    1.621422] x5 : 0000000000000040 x4 : 0000000000000000 x3 : 0000000000000001
[    1.628539] x2 : ffff000002ce0000 x1 : ffff000003d99410 x0 : 0000000000000003
[    1.635660] Call trace:
[    1.642766] iommu_create_device_direct_mappings
(drivers/iommu/iommu.c:1142 (discriminator 7)) (P)
[    1.645031] iommu_setup_default_domain (drivers/iommu/iommu.c:2992
(discriminator 1))
[    1.651277] iommu_device_register (drivers/iommu/iommu.c:1905
drivers/iommu/iommu.c:277)
[    1.655877] qcom_iommu_device_probe
(drivers/iommu/arm/arm-smmu/qcom_iommu.c:860)
[    1.660392] platform_probe (drivers/base/platform.c:1404)
[    1.665163] really_probe (drivers/base/dd.c:579 drivers/base/dd.c:657)
[    1.668723] __driver_probe_device (drivers/base/dd.c:799)
[    1.672371] driver_probe_device (drivers/base/dd.c:829)
[    1.676623] __driver_attach (drivers/base/dd.c:1216 drivers/base/dd.c:1155)
[    1.680615] bus_for_each_dev (drivers/base/bus.c:370)
[    1.684434] driver_attach (drivers/base/dd.c:1234)
[    1.688255] bus_add_driver (drivers/base/bus.c:678)
[    1.692073] driver_register (drivers/base/driver.c:249)
[    1.695633] __platform_driver_register (drivers/base/platform.c:868)
[    1.699368] qcom_iommu_init (drivers/iommu/arm/arm-smmu/qcom_iommu.c:943)
[    1.704226] do_one_initcall (init/main.c:1269)
[    1.707873] kernel_init_freeable (init/main.c:1330 (discriminator
1) init/main.c:1347 (discriminator 1) init/main.c:1366 (discriminator
1) init/main.c:1579 (discriminator 1))
[    1.711607] kernel_init (init/main.c:1473)
[    1.716118] ret_from_fork (arch/arm64/kernel/entry.S:863)
[    1.719419] ---[ end trace 0000000000000000 ]---
[    1.723302] iommu 1ef0000.iommu: IOMMU driver was not able to
establish FW requested direct mapping.
[    1.734231] platform 1c00000.gpu: Adding to iommu group 2
[    1.748218] loop: module loaded

Links:
 - https://lkft.validation.linaro.org/scheduler/job/8350682#L2838

> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> @@ -229,7 +229,7 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
>                 goto out_unlock;
>
>         pgtbl_cfg = (struct io_pgtable_cfg) {
> -               .pgsize_bitmap  = domain->pgsize_bitmap,
> +               .pgsize_bitmap  = SZ_4K | SZ_64K | SZ_1M | SZ_16M,
>                 .ias            = 32,
>                 .oas            = 40,
>                 .tlb            = &qcom_flush_ops,
> @@ -246,6 +246,8 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
>                 goto out_clear_iommu;
>         }
>
> +       /* Update the domain's page sizes to reflect the page table format */
> +       domain->pgsize_bitmap = pgtbl_cfg.pgsize_bitmap;
>         domain->geometry.aperture_end = (1ULL << pgtbl_cfg.ias) - 1;
>         domain->geometry.force_aperture = true;
>
> @@ -335,7 +337,6 @@ static struct iommu_domain *qcom_iommu_domain_alloc_paging(struct device *dev)
>
>         mutex_init(&qcom_domain->init_mutex);
>         spin_lock_init(&qcom_domain->pgtbl_lock);
> -       qcom_domain->domain.pgsize_bitmap = SZ_4K | SZ_64K | SZ_1M | SZ_16M;
>
>         return &qcom_domain->domain;
>  }
>
> Of all the drivers qcom is the only one that uses the 64 bit arm page
> table, 4 & 64k sizes, and was using the ops global. The io_pgtable
> code will remove one of the two depending on PAGE_SIZE which makes
> things inconsistent and hits that warn.
>
> Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: next-20250702 WARNING iommu io-pgtable-arm.c at arm_lpae_map_pages qcom_iommu_map
  2025-07-09 10:44   ` Naresh Kamboju
@ 2025-07-09 16:26     ` Jason Gunthorpe
  2025-07-09 16:50       ` Arnd Bergmann
  2025-07-10 10:38       ` Naresh Kamboju
  0 siblings, 2 replies; 7+ messages in thread
From: Jason Gunthorpe @ 2025-07-09 16:26 UTC (permalink / raw)
  To: Naresh Kamboju
  Cc: open list, iommu, lkft-triage, Linux Regressions, Nicolin Chen,
	Jean-Philippe Brucker, Anders Roxell, Ben Copeland, Arnd Bergmann,
	Dan Carpenter

[-- Attachment #1: Type: text/plain, Size: 1316 bytes --]

On Wed, Jul 09, 2025 at 04:14:26PM +0530, Naresh Kamboju wrote:
> I have tested this patch on top of Linux next-20250702 tag,
> and found kernel warning,

Oh, yeah, I guess that hacky fix is not going to work.

Then this is probably good enough (against linux-next):

--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -335,7 +335,7 @@ static struct iommu_domain *qcom_iommu_domain_alloc_paging(struct device *dev)
 
        mutex_init(&qcom_domain->init_mutex);
        spin_lock_init(&qcom_domain->pgtbl_lock);
-       qcom_domain->domain.pgsize_bitmap = SZ_4K | SZ_64K | SZ_1M | SZ_16M;
+       qcom_domain->domain.pgsize_bitmap = SZ_4K | SZ_2M;
 
        return &qcom_domain->domain;
 }

I believe the original text was a copy and pasto from an ARMv7s driver
(ie the 32 bit ARM page table) which uses that unique combination of
sizes. It is not a sane bitmap for HW with 64 bit page table support,
there is never a 1M option for instance.

So this removes 64k page support, which maybe didn't even work?

I also prepared a proper rework that puts the pgtable allocation into
the qcom_iommu_domain_alloc_paging(). I've attached it, but it is
involved enough it probably breaks something else. However if you
want to test it maybe we can progress it too.

Thanks,
Jason

[-- Attachment #2: 0001-iommu-qcom-Allocate-the-iopgtbl-inside-qcom_iommu_do.patch --]
[-- Type: text/x-diff, Size: 8837 bytes --]

From a86762cd05296949a8fc20bcb7558aa57b137cd2 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgg@nvidia.com>
Date: Wed, 9 Jul 2025 13:24:40 -0300
Subject: [PATCH] iommu/qcom: Allocate the iopgtbl inside
 qcom_iommu_domain_alloc_paging()

Structure the driver the way the current driver API expects.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 159 ++++++++++--------------
 1 file changed, 65 insertions(+), 94 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 5891ad5de0d5e2..f65c8b50903319 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -66,7 +66,6 @@ struct qcom_iommu_ctx {
 struct qcom_iommu_domain {
 	struct io_pgtable_ops	*pgtbl_ops;
 	spinlock_t		 pgtbl_lock;
-	struct mutex		 init_mutex; /* Protects iommu pointer */
 	struct iommu_domain	 domain;
 	struct qcom_iommu_dev	*iommu;
 	struct iommu_fwspec	*fwspec;
@@ -213,42 +212,16 @@ static irqreturn_t qcom_iommu_fault(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
-static int qcom_iommu_init_domain(struct iommu_domain *domain,
-				  struct qcom_iommu_dev *qcom_iommu,
-				  struct device *dev)
+static int qcom_iommu_attach_domain(struct qcom_iommu_domain *qcom_domain,
+				    struct device *dev)
 {
-	struct qcom_iommu_domain *qcom_domain = to_qcom_iommu_domain(domain);
-	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
-	struct io_pgtable_ops *pgtbl_ops;
-	struct io_pgtable_cfg pgtbl_cfg;
+	struct qcom_iommu_dev *qcom_iommu = qcom_domain->iommu;
+	struct iommu_fwspec *fwspec = qcom_domain->fwspec;
+	struct io_pgtable_cfg *pgtbl_cfg =
+		&io_pgtable_ops_to_pgtable(qcom_domain->pgtbl_ops)->cfg;
 	int i, ret = 0;
 	u32 reg;
 
-	mutex_lock(&qcom_domain->init_mutex);
-	if (qcom_domain->iommu)
-		goto out_unlock;
-
-	pgtbl_cfg = (struct io_pgtable_cfg) {
-		.pgsize_bitmap	= domain->pgsize_bitmap,
-		.ias		= 32,
-		.oas		= 40,
-		.tlb		= &qcom_flush_ops,
-		.iommu_dev	= qcom_iommu->dev,
-	};
-
-	qcom_domain->iommu = qcom_iommu;
-	qcom_domain->fwspec = fwspec;
-
-	pgtbl_ops = alloc_io_pgtable_ops(ARM_32_LPAE_S1, &pgtbl_cfg, qcom_domain);
-	if (!pgtbl_ops) {
-		dev_err(qcom_iommu->dev, "failed to allocate pagetable ops\n");
-		ret = -ENOMEM;
-		goto out_clear_iommu;
-	}
-
-	domain->geometry.aperture_end = (1ULL << pgtbl_cfg.ias) - 1;
-	domain->geometry.force_aperture = true;
-
 	for (i = 0; i < fwspec->num_ids; i++) {
 		struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain, fwspec->ids[i]);
 
@@ -256,14 +229,14 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 			ret = qcom_scm_restore_sec_cfg(qcom_iommu->sec_id, ctx->asid);
 			if (ret) {
 				dev_err(qcom_iommu->dev, "secure init failed: %d\n", ret);
-				goto out_clear_iommu;
+				return ret;
 			}
 			ctx->secure_init = true;
 		}
 
 		/* Secured QSMMU-500/QSMMU-v2 contexts cannot be programmed */
 		if (ctx->secured_ctx) {
-			ctx->domain = domain;
+			ctx->domain = &qcom_domain->domain;
 			continue;
 		}
 
@@ -276,21 +249,21 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 
 		/* TTBRs */
 		iommu_writeq(ctx, ARM_SMMU_CB_TTBR0,
-				pgtbl_cfg.arm_lpae_s1_cfg.ttbr |
+				pgtbl_cfg->arm_lpae_s1_cfg.ttbr |
 				FIELD_PREP(ARM_SMMU_TTBRn_ASID, ctx->asid));
 		iommu_writeq(ctx, ARM_SMMU_CB_TTBR1, 0);
 
 		/* TCR */
 		iommu_writel(ctx, ARM_SMMU_CB_TCR2,
-				arm_smmu_lpae_tcr2(&pgtbl_cfg));
+				arm_smmu_lpae_tcr2(pgtbl_cfg));
 		iommu_writel(ctx, ARM_SMMU_CB_TCR,
-			     arm_smmu_lpae_tcr(&pgtbl_cfg) | ARM_SMMU_TCR_EAE);
+			     arm_smmu_lpae_tcr(pgtbl_cfg) | ARM_SMMU_TCR_EAE);
 
 		/* MAIRs (stage-1 only) */
 		iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR0,
-				pgtbl_cfg.arm_lpae_s1_cfg.mair);
+				pgtbl_cfg->arm_lpae_s1_cfg.mair);
 		iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR1,
-				pgtbl_cfg.arm_lpae_s1_cfg.mair >> 32);
+				pgtbl_cfg->arm_lpae_s1_cfg.mair >> 32);
 
 		/* SCTLR */
 		reg = ARM_SMMU_SCTLR_CFIE | ARM_SMMU_SCTLR_CFRE |
@@ -303,58 +276,74 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 
 		iommu_writel(ctx, ARM_SMMU_CB_SCTLR, reg);
 
-		ctx->domain = domain;
+		ctx->domain = &qcom_domain->domain;
 	}
-
-	mutex_unlock(&qcom_domain->init_mutex);
-
-	/* Publish page table ops for map/unmap */
-	qcom_domain->pgtbl_ops = pgtbl_ops;
-
 	return 0;
-
-out_clear_iommu:
-	qcom_domain->iommu = NULL;
-out_unlock:
-	mutex_unlock(&qcom_domain->init_mutex);
-	return ret;
 }
 
 static struct iommu_domain *qcom_iommu_domain_alloc_paging(struct device *dev)
 {
+	struct qcom_iommu_dev *qcom_iommu = dev_iommu_priv_get(dev);
 	struct qcom_iommu_domain *qcom_domain;
+	struct io_pgtable_ops *pgtbl_ops;
+	struct io_pgtable_cfg pgtbl_cfg;
+	int ret;
 
-	/*
-	 * Allocate the domain and initialise some of its data structures.
-	 * We can't really do anything meaningful until we've added a
-	 * master.
-	 */
 	qcom_domain = kzalloc(sizeof(*qcom_domain), GFP_KERNEL);
 	if (!qcom_domain)
 		return NULL;
 
-	mutex_init(&qcom_domain->init_mutex);
 	spin_lock_init(&qcom_domain->pgtbl_lock);
-	qcom_domain->domain.pgsize_bitmap = SZ_4K | SZ_64K | SZ_1M | SZ_16M;
+
+	pgtbl_cfg = (struct io_pgtable_cfg) {
+		.pgsize_bitmap	= SZ_4K | SZ_64K | SZ_2M,
+		.ias		= 32,
+		.oas		= 40,
+		.tlb		= &qcom_flush_ops,
+		.iommu_dev	= qcom_iommu->dev,
+	};
+
+	qcom_domain->iommu = qcom_iommu;
+
+	/*
+	 * This driver doesn't keep track of devices attached to a domain,
+	 * so each domain is single device. The single fwspec is used
+	 * to push the invalidations.
+	 */
+	qcom_domain->fwspec = dev_iommu_fwspec_get(dev);
+
+	pgtbl_ops = alloc_io_pgtable_ops(ARM_32_LPAE_S1, &pgtbl_cfg, qcom_domain);
+	if (!pgtbl_ops) {
+		dev_err(qcom_iommu->dev, "failed to allocate pagetable ops\n");
+		ret = -ENOMEM;
+		goto err_free;
+	}
+
+	qcom_domain->domain.pgsize_bitmap = pgtbl_cfg.pgsize_bitmap;
+	qcom_domain->domain.geometry.aperture_end = (1ULL << pgtbl_cfg.ias) - 1;
+	qcom_domain->domain.geometry.force_aperture = true;
+	qcom_domain->pgtbl_ops = pgtbl_ops;
 
 	return &qcom_domain->domain;
+
+err_free:
+	kfree(qcom_domain);
+	return ERR_PTR(ret);
 }
 
 static void qcom_iommu_domain_free(struct iommu_domain *domain)
 {
 	struct qcom_iommu_domain *qcom_domain = to_qcom_iommu_domain(domain);
 
-	if (qcom_domain->iommu) {
-		/*
-		 * NOTE: unmap can be called after client device is powered
-		 * off, for example, with GPUs or anything involving dma-buf.
-		 * So we cannot rely on the device_link.  Make sure the IOMMU
-		 * is on to avoid unclocked accesses in the TLB inv path:
-		 */
-		pm_runtime_get_sync(qcom_domain->iommu->dev);
-		free_io_pgtable_ops(qcom_domain->pgtbl_ops);
-		pm_runtime_put_sync(qcom_domain->iommu->dev);
-	}
+	/*
+	 * NOTE: unmap can be called after client device is powered
+	 * off, for example, with GPUs or anything involving dma-buf.
+	 * So we cannot rely on the device_link.  Make sure the IOMMU
+	 * is on to avoid unclocked accesses in the TLB inv path:
+	 */
+	pm_runtime_get_sync(qcom_domain->iommu->dev);
+	free_io_pgtable_ops(qcom_domain->pgtbl_ops);
+	pm_runtime_put_sync(qcom_domain->iommu->dev);
 
 	kfree(qcom_domain);
 }
@@ -365,47 +354,29 @@ static int qcom_iommu_attach_dev(struct iommu_domain *domain, struct device *dev
 	struct qcom_iommu_domain *qcom_domain = to_qcom_iommu_domain(domain);
 	int ret;
 
-	if (!qcom_iommu) {
-		dev_err(dev, "cannot attach to IOMMU, is it on the same bus?\n");
-		return -ENXIO;
-	}
+	if (qcom_domain->iommu != qcom_iommu ||
+	    qcom_domain->fwspec != dev_iommu_fwspec_get(dev))
+		return -EINVAL;
 
 	/* Ensure that the domain is finalized */
 	pm_runtime_get_sync(qcom_iommu->dev);
-	ret = qcom_iommu_init_domain(domain, qcom_iommu, dev);
+	ret = qcom_iommu_attach_domain(qcom_domain, dev);
 	pm_runtime_put_sync(qcom_iommu->dev);
 	if (ret < 0)
 		return ret;
-
-	/*
-	 * Sanity check the domain. We don't support domains across
-	 * different IOMMUs.
-	 */
-	if (qcom_domain->iommu != qcom_iommu)
-		return -EINVAL;
-
 	return 0;
 }
 
 static int qcom_iommu_identity_attach(struct iommu_domain *identity_domain,
 				      struct device *dev)
 {
-	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
-	struct qcom_iommu_domain *qcom_domain;
 	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
 	struct qcom_iommu_dev *qcom_iommu = dev_iommu_priv_get(dev);
 	unsigned int i;
 
-	if (domain == identity_domain || !domain)
-		return 0;
-
-	qcom_domain = to_qcom_iommu_domain(domain);
-	if (WARN_ON(!qcom_domain->iommu))
-		return -EINVAL;
-
 	pm_runtime_get_sync(qcom_iommu->dev);
 	for (i = 0; i < fwspec->num_ids; i++) {
-		struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain, fwspec->ids[i]);
+		struct qcom_iommu_ctx *ctx = qcom_iommu->ctxs[fwspec->ids[i]];
 
 		/* Disable the context bank: */
 		iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: next-20250702 WARNING iommu io-pgtable-arm.c at arm_lpae_map_pages qcom_iommu_map
  2025-07-09 16:26     ` Jason Gunthorpe
@ 2025-07-09 16:50       ` Arnd Bergmann
  2025-07-09 17:22         ` Jason Gunthorpe
  2025-07-10 10:38       ` Naresh Kamboju
  1 sibling, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2025-07-09 16:50 UTC (permalink / raw)
  To: Jason Gunthorpe, Naresh Kamboju
  Cc: open list, iommu, lkft-triage, Linux Regressions, Nicolin Chen,
	Jean-Philippe Brucker, Anders Roxell, Benjamin Copeland,
	Dan Carpenter

On Wed, Jul 9, 2025, at 18:26, Jason Gunthorpe wrote:
> On Wed, Jul 09, 2025 at 04:14:26PM +0530, Naresh Kamboju wrote:
>
> I believe the original text was a copy and pasto from an ARMv7s driver
> (ie the 32 bit ARM page table) which uses that unique combination of
> sizes. It is not a sane bitmap for HW with 64 bit page table support,
> there is never a 1M option for instance.
>
> So this removes 64k page support, which maybe didn't even work?

My guess would be that this bug is specific to this SoC running
in 32-bit mode. This is a rare exception and not really well
supported, as most 64-bit Arm chips require a 64-bit kernel, but
this one (along with Broadcom bcm283x) can do either.

When running a 32-bit kernel, there is definitely no support for
64KB pages in the CPU, unlike on arm64.

    Arnd

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: next-20250702 WARNING iommu io-pgtable-arm.c at arm_lpae_map_pages qcom_iommu_map
  2025-07-09 16:50       ` Arnd Bergmann
@ 2025-07-09 17:22         ` Jason Gunthorpe
  0 siblings, 0 replies; 7+ messages in thread
From: Jason Gunthorpe @ 2025-07-09 17:22 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Naresh Kamboju, open list, iommu, lkft-triage, Linux Regressions,
	Nicolin Chen, Jean-Philippe Brucker, Anders Roxell,
	Benjamin Copeland, Dan Carpenter

On Wed, Jul 09, 2025 at 06:50:02PM +0200, Arnd Bergmann wrote:
> On Wed, Jul 9, 2025, at 18:26, Jason Gunthorpe wrote:
> > On Wed, Jul 09, 2025 at 04:14:26PM +0530, Naresh Kamboju wrote:
> >
> > I believe the original text was a copy and pasto from an ARMv7s driver
> > (ie the 32 bit ARM page table) which uses that unique combination of
> > sizes. It is not a sane bitmap for HW with 64 bit page table support,
> > there is never a 1M option for instance.
> >
> > So this removes 64k page support, which maybe didn't even work?
> 
> My guess would be that this bug is specific to this SoC running
> in 32-bit mode.

Sorry, it was unclear.

This driver always uses the ARM page table format with 64 bit
entries. It uses the ARM_32_LPAE_S1 sub-flavour of it.

This is different from the ARM page table format with 32 bit entries
called ARM_V7S. Only this format uses the 'SZ_4K | SZ_64K | SZ_1M |
SZ_16M' bitmap.

Looking more closely when a driver selects ARM_32_LPAE_S1 it
calls arm_32_lpae_alloc_pgtable_s1() which does:

	cfg->pgsize_bitmap &= (SZ_4K | SZ_2M | SZ_1G);

So the 1 line patch looks OKish (maybe drop the SZ_2M to be
conservative), despite putting it in the bitmap 64K would have never
be used in the iommu no matter what the CPU is doing.

Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: next-20250702 WARNING iommu io-pgtable-arm.c at arm_lpae_map_pages qcom_iommu_map
  2025-07-09 16:26     ` Jason Gunthorpe
  2025-07-09 16:50       ` Arnd Bergmann
@ 2025-07-10 10:38       ` Naresh Kamboju
  1 sibling, 0 replies; 7+ messages in thread
From: Naresh Kamboju @ 2025-07-10 10:38 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: open list, iommu, lkft-triage, Linux Regressions, Nicolin Chen,
	Jean-Philippe Brucker, Anders Roxell, Ben Copeland, Arnd Bergmann,
	Dan Carpenter

On Wed, 9 Jul 2025 at 21:56, Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Wed, Jul 09, 2025 at 04:14:26PM +0530, Naresh Kamboju wrote:
> > I have tested this patch on top of Linux next-20250702 tag,
> > and found kernel warning,
>
> Oh, yeah, I guess that hacky fix is not going to work.
>
> Then this is probably good enough (against linux-next):
>
> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> @@ -335,7 +335,7 @@ static struct iommu_domain *qcom_iommu_domain_alloc_paging(struct device *dev)
>
>         mutex_init(&qcom_domain->init_mutex);
>         spin_lock_init(&qcom_domain->pgtbl_lock);
> -       qcom_domain->domain.pgsize_bitmap = SZ_4K | SZ_64K | SZ_1M | SZ_16M;
> +       qcom_domain->domain.pgsize_bitmap = SZ_4K | SZ_2M;
>
>         return &qcom_domain->domain;
>  }

I have tested the above patch and it fixes the reported issue.

Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>

* https://lkft.validation.linaro.org/scheduler/job/8351756#L2565

--
Linaro LKFT
https://lkft.linaro.org

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-07-10 10:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-08 20:56 next-20250702 WARNING iommu io-pgtable-arm.c at arm_lpae_map_pages qcom_iommu_map Naresh Kamboju
2025-07-09  0:25 ` Jason Gunthorpe
2025-07-09 10:44   ` Naresh Kamboju
2025-07-09 16:26     ` Jason Gunthorpe
2025-07-09 16:50       ` Arnd Bergmann
2025-07-09 17:22         ` Jason Gunthorpe
2025-07-10 10:38       ` Naresh Kamboju

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).