diff mbox series

[v3,3/9] iommu/arm-smmu: Implement ->probe_finalize()

Message ID 20210603164632.1000458-4-thierry.reding@gmail.com (mailing list archive)
State New, archived
Headers show
Series arm64: tegra: Prevent early SMMU faults | expand

Commit Message

Thierry Reding June 3, 2021, 4:46 p.m. UTC 
From: Thierry Reding <treding@nvidia.com>

Implement a ->probe_finalize() callback that can be used by vendor
implementations to perform extra programming necessary after devices
have been attached to the SMMU.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- remove unnecessarily paranoid check

 drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 +++++++++++++
 drivers/iommu/arm/arm-smmu/arm-smmu.h |  1 +
 2 files changed, 14 insertions(+)

Comments

Marek Szyprowski June 15, 2021, 6:01 p.m. UTC | #1 
Hi,

On 03.06.2021 18:46, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Implement a ->probe_finalize() callback that can be used by vendor
> implementations to perform extra programming necessary after devices
> have been attached to the SMMU.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

This patch landed recently in linux-next as commit 0d97174aeadf 
("iommu/arm-smmu: Implement ->probe_finalize()"). It causes the 
following issue on ARM Juno R1 board:

arm-smmu 2b500000.iommu: probing hardware configuration...
arm-smmu 2b500000.iommu: SMMUv1 with:
arm-smmu 2b500000.iommu:         stage 2 translation
arm-smmu 2b500000.iommu:         coherent table walk
arm-smmu 2b500000.iommu:         stream matching with 32 register groups
arm-smmu 2b500000.iommu:         4 context banks (4 stage-2 only)
arm-smmu 2b500000.iommu:         Supported page sizes: 0x60211000
arm-smmu 2b500000.iommu:         Stage-2: 40-bit IPA -> 40-bit PA
arm-smmu 7fb00000.iommu: probing hardware configuration...
arm-smmu 7fb00000.iommu: SMMUv1 with:
arm-smmu 7fb00000.iommu:         stage 2 translation
arm-smmu 7fb00000.iommu:         coherent table walk
arm-smmu 7fb00000.iommu:         stream matching with 16 register groups
arm-smmu 7fb00000.iommu:         4 context banks (4 stage-2 only)
arm-smmu 7fb00000.iommu:         Supported page sizes: 0x60211000
arm-smmu 7fb00000.iommu:         Stage-2: 40-bit IPA -> 40-bit PA
arm-smmu 7fb10000.iommu: probing hardware configuration...
arm-smmu 7fb10000.iommu: SMMUv1 with:
arm-smmu 7fb10000.iommu:         stage 2 translation
arm-smmu 7fb10000.iommu:         non-coherent table walk
arm-smmu 7fb10000.iommu:         (IDR0.CTTW overridden by FW configuration)
arm-smmu 7fb10000.iommu:         stream matching with 2 register groups
arm-smmu 7fb10000.iommu:         1 context banks (1 stage-2 only)
arm-smmu 7fb10000.iommu:         Supported page sizes: 0x60211000
arm-smmu 7fb10000.iommu:         Stage-2: 40-bit IPA -> 40-bit PA
arm-smmu 7fb20000.iommu: probing hardware configuration...
arm-smmu 7fb20000.iommu: SMMUv1 with:
arm-smmu 7fb20000.iommu:         stage 2 translation
arm-smmu 7fb20000.iommu:         non-coherent table walk
arm-smmu 7fb20000.iommu:         (IDR0.CTTW overridden by FW configuration)
arm-smmu 7fb20000.iommu:         stream matching with 2 register groups
arm-smmu 7fb20000.iommu:         1 context banks (1 stage-2 only)
arm-smmu 7fb20000.iommu:         Supported page sizes: 0x60211000
arm-smmu 7fb20000.iommu:         Stage-2: 40-bit IPA -> 40-bit PA
arm-smmu 7fb30000.iommu: probing hardware configuration...
arm-smmu 7fb30000.iommu: SMMUv1 with:
arm-smmu 7fb30000.iommu:         stage 2 translation
arm-smmu 7fb30000.iommu:         coherent table walk
arm-smmu 7fb30000.iommu:         stream matching with 2 register groups
arm-smmu 7fb30000.iommu:         1 context banks (1 stage-2 only)
arm-smmu 7fb30000.iommu:         Supported page sizes: 0x60211000
arm-smmu 7fb30000.iommu:         Stage-2: 40-bit IPA -> 40-bit PA
tda998x 0-0070: found TDA19988
tda998x 0-0071: found TDA19988
brd: module loaded
loop: module loaded
megasas: 07.714.04.00-rc1
sata_sil24 0000:03:00.0: Adding to iommu group 0
Unable to handle kernel NULL pointer dereference at virtual address 
0000000000000070
Mem abort info:
   ESR = 0x96000004
   EC = 0x25: DABT (current EL), IL = 32 bits
   SET = 0, FnV = 0
   EA = 0, S1PTW = 0
Data abort info:
   ISV = 0, ISS = 0x00000004
   CM = 0, WnR = 0
[0000000000000070] user address but active_mm is swapper
Internal error: Oops: 96000004 [#1] PREEMPT SMP
Modules linked in:
CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc1+ #3466
Hardware name: ARM Juno development board (r1) (DT)
pstate: 20000005 (nzCv daif -PAN -UAO -TCO BTYPE=--)
pc : arm_smmu_probe_finalize+0x14/0x48
lr : iommu_probe_device+0x74/0x120
...
Call trace:
  arm_smmu_probe_finalize+0x14/0x48
  of_iommu_configure+0xe4/0x1b8
  of_dma_configure_id+0xf8/0x2d8
  pci_dma_configure+0x44/0x88
  really_probe+0xc0/0x3c0
  driver_probe_device+0x60/0xc0
  device_driver_attach+0x6c/0x78
  __driver_attach+0xc0/0x100
  bus_for_each_dev+0x68/0xc8
  driver_attach+0x20/0x28
  bus_add_driver+0x168/0x1f8
  driver_register+0x60/0x110
  __pci_register_driver+0x5c/0x68
  sil24_pci_driver_init+0x20/0x28
  do_one_initcall+0x84/0x450
  kernel_init_freeable+0x2dc/0x334
  kernel_init+0x10/0x110
  ret_from_fork+0x10/0x18
Code: b40001e1 f9405821 f9400023 f9401461 (f9403822)
---[ end trace 561eda4b855861d1 ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
SMP: stopping secondary CPUs
Kernel Offset: disabled
CPU features: 0x00240022,25006086
Memory Limit: none
---[ end Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x0000000b ]---

> ---
> Changes in v2:
> - remove unnecessarily paranoid check
>
>   drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 +++++++++++++
>   drivers/iommu/arm/arm-smmu/arm-smmu.h |  1 +
>   2 files changed, 14 insertions(+)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 6f72c4d208ca..d20ce4d57df2 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -1450,6 +1450,18 @@ static void arm_smmu_release_device(struct device *dev)
>   	iommu_fwspec_free(dev);
>   }
>   
> +static void arm_smmu_probe_finalize(struct device *dev)
> +{
> +	struct arm_smmu_master_cfg *cfg;
> +	struct arm_smmu_device *smmu;
> +
> +	cfg = dev_iommu_priv_get(dev);
> +	smmu = cfg->smmu;
> +
> +	if (smmu->impl->probe_finalize)
> +		smmu->impl->probe_finalize(smmu, dev);
> +}
> +
>   static struct iommu_group *arm_smmu_device_group(struct device *dev)
>   {
>   	struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
> @@ -1569,6 +1581,7 @@ static struct iommu_ops arm_smmu_ops = {
>   	.iova_to_phys		= arm_smmu_iova_to_phys,
>   	.probe_device		= arm_smmu_probe_device,
>   	.release_device		= arm_smmu_release_device,
> +	.probe_finalize		= arm_smmu_probe_finalize,
>   	.device_group		= arm_smmu_device_group,
>   	.enable_nesting		= arm_smmu_enable_nesting,
>   	.set_pgtable_quirks	= arm_smmu_set_pgtable_quirks,
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index c31a59d35c64..147c95e7c59c 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -439,6 +439,7 @@ struct arm_smmu_impl {
>   				  struct device *dev, int start);
>   	void (*write_s2cr)(struct arm_smmu_device *smmu, int idx);
>   	void (*write_sctlr)(struct arm_smmu_device *smmu, int idx, u32 reg);
> +	void (*probe_finalize)(struct arm_smmu_device *smmu, struct device *dev);
>   };
>   
>   #define INVALID_SMENDX			-1

Best regards
Robin Murphy June 15, 2021, 6:08 p.m. UTC | #2 
On 2021-06-15 19:01, Marek Szyprowski wrote:
> Hi,
> 
> On 03.06.2021 18:46, Thierry Reding wrote:
>> From: Thierry Reding <treding@nvidia.com>
>>
>> Implement a ->probe_finalize() callback that can be used by vendor
>> implementations to perform extra programming necessary after devices
>> have been attached to the SMMU.
>>
>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> 
> This patch landed recently in linux-next as commit 0d97174aeadf
> ("iommu/arm-smmu: Implement ->probe_finalize()"). It causes the
> following issue on ARM Juno R1 board:

[...]

>> +static void arm_smmu_probe_finalize(struct device *dev)
>> +{
>> +	struct arm_smmu_master_cfg *cfg;
>> +	struct arm_smmu_device *smmu;
>> +
>> +	cfg = dev_iommu_priv_get(dev);
>> +	smmu = cfg->smmu;
>> +
>> +	if (smmu->impl->probe_finalize)

Oops, indeed that needs to check smmu->impl first.

Robin.

>> +		smmu->impl->probe_finalize(smmu, dev);
>> +}
>> +
Krishna Reddy June 15, 2021, 6:12 p.m. UTC | #3 
> if (smmu->impl->probe_finalize)

The above is the issue. It should be updated as below similar to other instances impl callbacks.
if (smmu->impl && smmu->impl->probe_finalize)

-KR
Will Deacon June 15, 2021, 6:21 p.m. UTC | #4 
On Tue, Jun 15, 2021 at 06:12:13PM +0000, Krishna Reddy wrote:
> > if (smmu->impl->probe_finalize)
> 
> The above is the issue. It should be updated as below similar to other instances impl callbacks.
> if (smmu->impl && smmu->impl->probe_finalize)

I'll push a patch on top shortly...

Will
Will Deacon June 15, 2021, 7:50 p.m. UTC | #5 
On Tue, Jun 15, 2021 at 07:21:35PM +0100, Will Deacon wrote:
> On Tue, Jun 15, 2021 at 06:12:13PM +0000, Krishna Reddy wrote:
> > > if (smmu->impl->probe_finalize)
> > 
> > The above is the issue. It should be updated as below similar to other instances impl callbacks.
> > if (smmu->impl && smmu->impl->probe_finalize)
> 
> I'll push a patch on top shortly...

Done:

https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-thierry/arm-smmu

I'll send this lot to Joerg tomorrow.

Thierry -- feel free to pull in the updated branch if you want the fix
sooner, as it may be a few days before this hits -next.

Cheers,

Will
diff mbox series

Patch

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 6f72c4d208ca..d20ce4d57df2 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1450,6 +1450,18 @@  static void arm_smmu_release_device(struct device *dev)
 	iommu_fwspec_free(dev);
 }
 
+static void arm_smmu_probe_finalize(struct device *dev)
+{
+	struct arm_smmu_master_cfg *cfg;
+	struct arm_smmu_device *smmu;
+
+	cfg = dev_iommu_priv_get(dev);
+	smmu = cfg->smmu;
+
+	if (smmu->impl->probe_finalize)
+		smmu->impl->probe_finalize(smmu, dev);
+}
+
 static struct iommu_group *arm_smmu_device_group(struct device *dev)
 {
 	struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
@@ -1569,6 +1581,7 @@  static struct iommu_ops arm_smmu_ops = {
 	.iova_to_phys		= arm_smmu_iova_to_phys,
 	.probe_device		= arm_smmu_probe_device,
 	.release_device		= arm_smmu_release_device,
+	.probe_finalize		= arm_smmu_probe_finalize,
 	.device_group		= arm_smmu_device_group,
 	.enable_nesting		= arm_smmu_enable_nesting,
 	.set_pgtable_quirks	= arm_smmu_set_pgtable_quirks,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index c31a59d35c64..147c95e7c59c 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -439,6 +439,7 @@  struct arm_smmu_impl {
 				  struct device *dev, int start);
 	void (*write_s2cr)(struct arm_smmu_device *smmu, int idx);
 	void (*write_sctlr)(struct arm_smmu_device *smmu, int idx, u32 reg);
+	void (*probe_finalize)(struct arm_smmu_device *smmu, struct device *dev);
 };
 
 #define INVALID_SMENDX			-1