[PATCH 1/3] iommu/arm-smmu: Make instance lookup robust

[Will Deacon]

On Fri, Nov 08, 2024 at 06:30:15PM +0000, Robin Murphy wrote:
> Relying on the driver list was a cute idea for minimising the scope of
> our SMMU device lookups, however it turns out to have a subtle flaw. The
> SMMU device only gets added to that list after arm_smmu_device_probe()
> returns success, so there's actually no way the iommu_device_register()
> call from there could ever work as intended, even if it wasn't already
> hampered by the fwspec setup not happening early enough.
> 
> Switch both arm_smmu_get_by_fwnode() implementations to use a platform
> bus lookup instead, which *will* reliably work. Also make sure that we
> don't register SMMUv2 instances until we've fully initialised them, to
> avoid similar consequences of the lookup now finding a device with no
> drvdata. Moving that code is also a perfect excuse to give it a little
> dev_err_probe() tidyup as well.
> 
> Signed-off-by: Robin Murphy <robin.murphy at arm.com>
> ---
> 
> This should in theory replace 229e6ee43d2a ("iommu/arm-smmu: Defer
> probe of clients after smmu device bound"), or at least allow it to be
> reverted going forward.

Let's revert that as part of this series, then?

>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  4 +--
>  drivers/iommu/arm/arm-smmu/arm-smmu.c       | 31 ++++++++++-----------
>  2 files changed, 16 insertions(+), 19 deletions(-)

[...]

> @@ -2255,6 +2240,18 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>  	arm_smmu_device_reset(smmu);
>  	arm_smmu_test_smr_masks(smmu);
>  
> +	err = iommu_device_sysfs_add(&smmu->iommu, smmu->dev, NULL,
> +				     "smmu.%pa", &smmu->ioaddr);
> +	if (err)
> +		return dev_err_probe(dev, err, "Failed to register iommu in sysfs\n");
> +
> +	err = iommu_device_register(&smmu->iommu, &arm_smmu_ops,
> +				    using_legacy_binding ? NULL : dev);
> +	if (err) {
> +		iommu_device_sysfs_remove(&smmu->iommu);
> +		return dev_err_probe(dev, err, "Failed to register iommu\n");

Why is dev_err_probe() better here? I've not seen it before, so I was
trying to figure out what it does. It mainly looks to be handling
the EPROBE_DEFER case, but after your other changes to use
bus_find_device_by_fwnode() I'm wondering whether we actually hit that
(particularly in iommu_device_sysfs_add())?

I'm not saying there's a problem, just that I'm not sure why
dev_err_probe() is tidier than dev_err().

Will