On 01/04/2025 3:11 pm, Geert Uytterhoeven wrote:
Hi Robin,
On Tue, 1 Apr 2025 at 15:53, Robin Murphy <robin.murphy@xxxxxxx> wrote:
On 2025-03-25 3:26 pm, Geert Uytterhoeven wrote:
On Thu, 20 Mar 2025 at 15:41, Robin Murphy <robin.murphy@xxxxxxx> wrote:
IPMMU registers almost-initialised instances, but misses assigning the
drvdata to make them fully functional, so initial calls back into
ipmmu_probe_device() are likely to fail unnecessarily. Reorder this to
work as it should, also pruning the long-out-of-date comment and adding
the missing sysfs cleanup on error for good measure.
Reported-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
Fixes: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path")
Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
Thanks for your patch!
This fixes the
sata_rcar ee300000.sata: late IOMMU probe at driver bind,
something fishy here!
WARNING: CPU: 1 PID: 13 at drivers/iommu/iommu.c:571
__iommu_probe_device+0x208/0x38c
I saw on Salvator-XS with R-Car M3-N.
It does not fix the second issue reported, so it is indeed too early for a
"Closes: https://lore.kernel.org/CAMuHMdWPFnHTFeeWL2-BU8tKOL-E5K2ROOz=LLBLTJJLCK9NgA@xxxxxxxxxxxxxx";
tag.
You mean .of_xlate being called multiple times? That's not an issue,
it's normal and expected. Every time an IOMMU instance registers, it
triggers a probe of all relevant devices which do not yet have an IOMMU
- this has never been selective, so if a device is associated with a
different already-registered IOMMU instance, but does not have a group
because that instance's .probe_device rejected it, that probe also gets
tried (and rejected) again.
The core code behaviour has been this way for a very long time, the only
new thing is that the .of_xlate calls are now in sync with their
corresponding .probe_device calls (and the latter are also now working
properly again for fwspec-based ops).
Hmm, I started seeing the extra calls only after bcb81ac6ae3c,
i.e. not since a very long time?
OK, maybe not "very" - there are a lot of pieces in this puzzle that
have all moved underfoot over time, but the point I was getting at is
that although fwspec-based drivers *are* only now seeing those extra
.probe_device calls, the design of the core code has been *trying* to
make them since at least 3-ish years ago when the last of
bus_set_iommu() went away.
Conversely though, you should also now *not* be seeing extra calls where
you weren't looking for them. I bet if you were to build the DMA driver
as a module and load/unload it a bunch of times, you would have seen
more repeated calls to .of_xlate/.probe_device, whereas now you
(correctly) won't.
Was it just that, or is there still something functionally amiss?
That's all for now ;-)
Great!
Thanks,
Robin.
